Skip to content

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

Merged
h-mayorquin merged 3 commits into
NeuralEnsemble:masterfrom
h-mayorquin:fix_CI
Oct 28, 2026
Merged

h-mayorquin merged 3 commits 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

AxelNoun commented Oct 28, 2026 •

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 each entry of entities_to_download, about 90 times per job by a static count (37 in rawiotest, about 50 in iotest), and each call now runs update against gin. With the 403s that is cheap, since 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 git ls-remote took about 134 to 136 s. On 2026-08-19 (run 32255394808), datalad install waited 135 s per candidate URL in that mode, after the hash step had succeeded. I reproduced the mode locally (Linux, the server never answering the connection attempt): git ls-remote and git fetch both gave up after 135 s. Each update would then wait as long, about 90 × 135 s ≈ 3 h 20 per job, still under the default 6 h job limit. 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.

@zm711 zm711 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is worth a try. Since I’m bad at actions in general I’ll trust you Heberto and we can tweak as necessary.

@h-mayorquin

Copy link
Copy Markdown
Contributor Author

@AxelNoun and @zm711 thanks for the review

I agree with the comments above and now:

  • The check now treats "impossible" like "error".
  • download_dataset updates each local folder at most once per process. Each pytest call asks gin once instead of about 90 times. When gin does not answer, a job waits for one timeout per pytest call instead of one per test class.

I checked both by hand on local datalad datasets, one with an unreachable remote and one with two remotes and no tracking branch, which is one of the cases that gives "impossible".

I also went through the CI history to see how often the timeout happens. Since August four test runs waited about 135 s on almost every download_dataset call and took between 30 and 60 minutes, for example:
https://github.com/NeuralEnsemble/python-neo/actions/runs/32055984529

The run of this PR today already went through the fallback. In the 3.14 job gin answered the hash step with a 403, the most recent cache was restored, the 93 updates failed with a 403 and all the tests passed:
https://github.com/NeuralEnsemble/python-neo/actions/runs/40714814173

@zm711 zm711 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merge this as soon as tests pass. This will be crucial for all our other PRs :)

@h-mayorquin
h-mayorquin merged commit c5d0cfb into NeuralEnsemble:master Oct 28, 2026
146 of 147 checks passed
@h-mayorquin
h-mayorquin deleted the fix_CI branch October 28, 2026 18:50
@zm711

zm711 commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

We need to fix one more thing. RTD builds still failing. But I don’t think they have access to a cache right? So we just let our docs fail or we remove any g-node from the docs. Maybe we only use g-node for releases and force them to eventually pass like I did last release? Maybe I’ll open an issue?

@h-mayorquin

Copy link
Copy Markdown
Contributor Author

Let's open an issue and take a look at that. I am unsure how the docs use gin. The will not be able to use the cache of github so most likely we should avoid them accessing gin.

@AxelNoun

Copy link
Copy Markdown
Contributor

On how the docs use gin: five places, six distinct files, about 30 MiB in total.

  1. doc/source/rawio.rst, a suppressed ipython block: plexon/File_plexon_2.plx, plexon/File_plexon_3.plx, blackrock/FileSpec2.3001.nev, blackrock/FileSpec2.3001.ns5
  2. examples/plot_read_files_neo_io.py: plexon/File_plexon_3.plx, spike2/File_spike2_1.smr
  3. examples/plot_read_files_neo_rawio.py: plexon/File_plexon_3.plx, plexon/File_plexon_2.plx
  4. examples/plot_read_proxy_with_lazy_load.py: micromed/File_micromed_1.TRC
  5. examples/plot_with_matplotlib.py: plexon/File_plexon_3.plx

All five call urlretrieve on web.gin.g-node.org at build time, and no cache can help: each RTD build starts from a fresh shallow clone and reinstalls the environment, so nothing a previous build downloaded survives.

The consequence is not limited to a red check. The 0.14.6 docs were never published: /en/stable/ still serves 0.14.5 and /en/0.14.6/ returns 404. That build failed on release day (35166503) and again on today's rebuild at 14:39 (35500224), both with five HTTP Error 403: Forbidden from gin, fatal in rawio.rst. Master failed the same way three times in October, out of 17 builds, so the failure is intermittent. From a residential address those files download normally, which suggests a filter on cloud ranges rather than an outage, consistent with #1897.

Keeping the docs off gin seems right to me. Mirroring those six files, as a release asset or a Zenodo record, leaves the tests on gin untouched and keeps the examples runnable for readers who copy them; freezing the outputs into static blocks would also work, at the cost of no longer exercising the IOs. I can open the issue with this inventory, and send the PR that switches url_repo once you decide where the files should live.

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.

3 participants