Skip to content

Fall back to the cached test data when gin cannot be reached in CI - #1938

Open
h-mayorquin wants to merge 1 commit into
NeuralEnsemble:masterfrom
h-mayorquin:fix_CI
Open

h-mayorquin wants to merge 1 commit into
NeuralEnsemble:masterfrom
h-mayorquin:fix_CI

Conversation

@h-mayorquin

Copy link
Copy Markdown
Contributor

Right now the IO tests fail before running whenever gin answers 403 to the runner, as on #1924, #1925 and #1926 on October 27. #1912 made the hash step fail so that no cache is built with an empty key, but the test workflows only restore caches, and even with a restored cache the tests failed on the same 403 because download_dataset always updates from gin (see #1896).

This PR follows what we do in SpikeInterface. The hash check now fails only in caches_cron_job.yml, where the cache is built, and io-test.yml and plexon2-testing.yml warn and restore the most recent cache through their existing restore-keys, as in SpikeInterface/spikeinterface#4772. download_dataset now warns and uses the local copy when the update from gin fails. SpikeInterface goes further and never updates an existing copy and checks each file against its annex MD5 instead (SpikeInterface/spikeinterface#2937), but I kept the update here so that a pull request still picks up data merged on gin before the next cron run. I checked the fallback by hand with a local dataset whose remote points to an unreachable address: on master download_dataset raises IncompleteResultsError, with this change it warns and returns the local data. @AxelNoun, as you wrote #1912, does this look right to you?

@AxelNoun

Copy link
Copy Markdown
Contributor

Yes, this looks right to me. What #1912 needed was that no cache gets saved under an empty key, and that check stays in caches_cron_job.yml. In the test workflows the exit 1 only moved the failure earlier, as its "Cost" section said.

Three things I checked:

  • The comment "the key matches no cache" relies on the Linux-datasets- entry from 2026-09-27 being gone. It is: gh cache list -R NeuralEnsemble/python-neo --key Linux-datasets- now lists only Linux-datasets-43e19611…. The cron still exits before saving, so no new entry with an empty key can appear. An empty hash therefore finds no cache under the exact key, and the prefix restore picks the most recent entry, as intended.
  • In datalad (I read 1.7.2), update yields status="error" when the fetch fails, so your check catches the 403 case. It also yields status="impossible" in a few other cases (e.g. "Could not determine update target"), and the default on_failure raises IncompleteResultsError for both. With on_failure="ignore" and a check on "error" only, an "impossible" result would now pass without a warning. Checking result["status"] in ("impossible", "error") would keep the previous coverage.
  • download_dataset is called in setUpClass for every entity of every test class, so a job where gin is unreachable now runs update against gin hundreds of times. With the 403s that is cheap (each fetch fails fast). But CI: fail when the ephy_testing_data HEAD hash cannot be resolved #1912 also saw a timeout mode on the cron, where a single git ls-remote took about 134 to 136 s. If git fetch behaves the same way in that mode (I have not checked this), each call would wait as long, and the job could run for hours. A module-level flag set on the first failed update, so that later calls in the same process skip the update, would bound this to one attempt per pytest invocation (two per job).

One limit, not a blocker: the cron builds the cache with datalad install --recursive --get-data, so the restored cache holds all file content as of that run. When gin refuses the whole job, data added on gin after the last cron run will still fail on 403. That seems the right trade-off to me.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants