You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Say which upstream is down when an importer fails - #3729
Adds a notify job to the import workflow, so that a failed import says what broke instead of leaving a red square to be read through.
The code it runs lives in research-software-ecosystem/utils#68, as notify-upstream-failures/, packaged like the importer actions this workflow already calls. Merge that one first, since this pins to @main.
Why
This run is the motivating case: bio.tools was down, the job went red, and telling that apart from a bug in the importer meant reading the log. Nobody is watching the log. The import is weekly, so a quiet failure can sit for a week or a month.
What the job does
For each importer that failed, the action re-probes that importer's upstream and reaches one of two verdicts:
verdict
meaning
upstream-unavailable
The resource is down. Nothing to fix; the importer stopped before writing, so data/ still holds the previous import and the next run resumes by itself.
needs-attention
The resource answers normally, so the fault is more likely in the importer or in a change to the resource's API.
The verdict is carried in one open issue per resource, labelled upstream-outage. A new failure opens it, a change of verdict comments, an unchanged verdict stays silent so a long outage does not alert weekly, and a recovery comments and closes it. Every run writes an outcome table to the job summary either way, so a green run still shows what was checked.
Notes on the job itself
if: always(). With the default, one failed importer would skip the notifier — precisely when it is wanted.
permissions. A job-level map denies every scope it does not name. issues: write for the tracking issue, actions: read for the stale-run check. No contents: there is no checkout, because the action brings its own script.
concurrency. find-then-create is not atomic across runners, so a manual dispatch overlapping the schedule could otherwise open duplicate issues for one outage.
Parallel to merge-imports, so it neither delays nor depends on the merge.
History
The script first lived here in .github/scripts. That is not where this repository keeps that kind of code — every importer this job reports on is a folder in utils with an action.yml — so it moved there, with its 27 tests and the workflow that runs them. What remains here is the call.
One consequence worth flagging: the tests used to read this file and check the probe table against it, and from utils they cannot. That check now happens at runtime instead, against the needs this job passes in. If you add an importer to this workflow and to this job's needs without adding it to the action's table, the notifier reports it as unprobed and goes red when it fails — rather than an unwatched importer failing in silence.
When the bio.tools import failed on 2026-09-27, the only signal was a
GitHub email to whoever last touched the cron, saying "import data" had
failed, and a log whose visible error was a jq message about a glob. Working
out that bio.tools itself was down, and that nothing in this repository
needed fixing, took reading the run log and probing the API by hand.
Adds a notify job that answers that question automatically. It needs all nine
importers, runs with if: always(), and for each importer that failed it
re-requests that importer's own upstream and distinguishes two cases:
upstream-unavailable the resource is not answering. Nothing to fix; the
next run resumes once it is back.
needs-attention the resource answers normally, so the failure is
probably in the importer or in an API change.
The verdict is carried in one open issue per resource, labelled
upstream-outage and mentioning @research-software-ecosystem/rsec-ops. A new
failure opens it, a change of verdict comments and refreshes it, an unchanged
verdict stays silent so that a long outage does not produce a weekly alert,
and a success comments and closes it. Every run also writes a table of
importer outcomes to the step summary, so the run page says what happened
without anyone opening a log.
Deciding this by probing rather than by having the importers classify their
own failures keeps the change to one job and one script: no new exit-code
convention, no per-importer plumbing in utils, and it tests the actual claim
at the moment of notification. The cost is that a blip which has recovered by
then reads as needs-attention, so the message always carries the probe result
alongside the verdict and lets the reader judge.
Probe targets are each importer's real upstream, read out of the importers
rather than guessed. Debian Med's is a PostgreSQL endpoint rather than a URL,
so tcp:// targets are supported and connect to udd-mirror.debian.net:5432.
HEAD is tried before GET because one target is a multi-megabyte archive and
only the status line matters. A 4xx still counts as reachable: a status line
means the host is serving, whereas 5xx, Cloudflare's 52x and a refused
connection do not.
Stdlib only, so the thing whose job is to report breakage has no dependency
that can break, and no pip step. Needs no secrets beyond the default
GITHUB_TOKEN with issues: write, which is granted on this job alone.
Verified against the real upstreams while bio.tools is down: nine probes,
15.7 s, bio.tools reported HTTP 521 -> upstream-unavailable and the other
eight reachable -> needs-attention, including the UDD TCP probe. The issue
lifecycle was checked against a stubbed API over six cases: first failure
opens, unchanged verdict stays quiet, changed verdict comments and refreshes,
recovery comments and closes, success with nothing open does nothing, and a
skipped job is not evaluated.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three findings from the review, all of them correct on the facts.
The permissions one was blocking: the job declared only `issues: write`, and a
job-level permissions map sets every scope it does not name to none, so
actions/checkout had no contents access and the job would have failed at the
checkout step before the notifier ever ran. The notifier would never have
reported anything. `contents: read` is now spelled out.
The repository has no `upstream-outage` label, which was confirmed against its
label list. Rather than depend on what GitHub does with an unknown label on
issue creation, the notifier now ensures the label exists before opening
anything, and only when an importer has actually failed. find_issue() filters
on that label, so an issue created without it would be invisible on the next
run and a duplicate would be opened for an outage already tracked.
find-then-create is not atomic, so a manual dispatch overlapping the schedule
could have both runs open an issue for the same outage. Re-checking
immediately before creating would narrow that window without closing it, since
the two runs are on different runners, so the job takes a concurrency group
instead and they serialise.
Worth noting separately: import.yaml has no concurrency control at all, so two
overlapping runs would already collide over the shared *-import-branch names,
well before the notifier is reached. That is out of scope here but wants its
own change.
Verified: the six issue-lifecycle cases still pass, and a dry run against the
live upstreams behaves correctly, now reporting bio.tools as reachable and the
verdict as needs-attention since the outage has ended.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two observations from the review summary, which filed no inline findings but
named both. Checked against what the importers actually fetch; one was right,
one was not.
Right: workflowhub-import reads the galaxy_codex tools.json as well as
workflowhub.eu, to map Galaxy tool IDs onto bio.tools IDs, and only
workflowhub.eu was probed. An outage at raw.githubusercontent.com fails that
importer while the probe reported the resource healthy, producing exactly the
wrong verdict. It is probed now. galaxytool-import fetches both tools.json and
workflows.json, so both are listed, though they share a host.
Not right: training.galaxyproject.org is not a dependency of
galaxytool-import. It appears there only in string construction for tutorial
links, and the importer's only network calls are the two galaxy_codex URLs.
Probing it would have reported an outage that could not affect the import.
Having more than one target per source made the verdict rule wrong: it said
needs-attention when *any* target answered. An importer that reads two
resources fails if either is down, so one reachable host is not evidence that
the fault is ours. It now requires all of them to answer.
Also right: an exception while reporting on one resource aborted the loop, so
the remaining importers went unreported and the step summary was never
written. The per-resource work moves into handle(), each call is isolated, a
failure is recorded in its row and in the exit status, and the summary is
written either way. The notifier exists to report failures, so it should not
be the most brittle job in the workflow.
Verified: three sources where the middle one raises still report all three,
the third is processed after the second blows up, and the run exits non-zero;
a two-target source reads as upstream-unavailable when either target is down
and needs-attention only when both answer; the six lifecycle cases still pass;
and a live dry run exercises all eleven targets.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The overview filed no inline findings but named two things; both were worth checking, and one of them was right.
"Several upstream dependencies are incompletely probed" — partly. I checked each importer's actual network calls:
workflowhub-import also reads the galaxy_codex tools.json, to map Galaxy tool IDs onto bio.tools IDs, and only workflowhub.eu was being probed. An outage at raw.githubusercontent.com would have failed that importer while the probe called the resource healthy — precisely the wrong verdict. Now probed.
galaxytool-import fetches both tools.json and workflows.json; both are listed now, though they share a host.
training.galaxyproject.org is not a dependency. It appears in galaxytool-import only in string construction for tutorial links; the importer's only network calls are the two galaxy_codex URLs. Probing it would have reported outages that cannot affect the import.
Having more than one target per source then exposed a bug in the verdict rule: it returned needs-attention when any target answered. An importer reading two resources fails if either is down, so one reachable host isn't evidence the fault is ours. It now requires all targets to answer.
"One notification failure can suppress later results" — correct. An exception while reporting on one resource aborted the loop, so the remaining importers went unreported and the step summary was never written. Per-resource work now lives in handle(), each call is isolated, a failure is recorded in its row and in the exit status, and the summary is written regardless. A job whose purpose is reporting failures shouldn't be the most brittle one in the workflow.
Verified: with three sources where the middle one raises, all three are still reported, the third is processed after the second fails, and the run exits non-zero; a two-target source reads upstream-unavailable when either target is down and needs-attention only when both answer; the six lifecycle cases still pass; and a live dry run exercises all eleven targets.
Probe the same HTTPS endpoint used by the importer
.github/scripts/notify_upstream_failures.py:36
The importer itself requests https://biii.eu, but this notifier probes http://biii.eu/. That makes the verdict depend on a different endpoint: if the HTTP listener/redirect is unavailable while HTTPS is healthy, a successful importer would be reported as upstream-unavailable. Probe the same HTTPS URL used by the importer.
Four more from the review, all of them correct.
The issues endpoint returns pull requests as well as issues. A PR that
happened to carry this title and label would have been picked up as the
tracking issue, and the recovery path would then have commented on it and
closed it. Entries with a pull_request key are skipped.
The BIII probe used http://biii.eu while the importer is invoked with
`-td https://biii.eu` in its action.yml; the http:// form in the script is an
RDF namespace, not a fetch target. The probe now uses the scheme the importer
uses, so an http listener problem can no longer be reported as an outage of a
resource that is serving.
The concurrency group serialises notifier runs but does not order them, so a
slow older run could still report after a newer one and reopen an outage the
newer run had just closed. The notifier now checks whether a newer run of this
workflow has started and, if so, writes its summary and makes no changes.
This is a symptom of import.yaml having no concurrency control of its own --
two overlapping runs already share the *-import-branch names, which is a
larger problem than a mis-filed issue -- but the notifier should not add to it.
ensure_label() ran outside the failure-tolerant path, so any non-404 error
reading or creating the label aborted main() before the summary was written.
That contradicted the guarantee added in the previous commit, and did so
precisely when the reporting path was already in trouble. Label setup is now
recorded like any other reporting failure.
Verified: a PR sharing the title is skipped and a PR-only result yields no
match; a run that is not the newest writes its summary and mutates nothing,
while the newest one still does; a 403 on label setup still produces the
summary and still exits non-zero. Earlier suites pass, and a live dry run
exercises all eleven targets.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The "previously missed" note about the BIII probe is also correct, and it was my error of exactly the kind I rejected a round earlier.
biii-import/action.yml invokes the script with -td https://biii.eu. The http://biii.eu in biseEU_LD_export.py is ns, an RDF namespace string — not a fetch target, the same category as training.galaxyproject.org in the Galaxy importer. I correctly declined to probe that one, then probed the wrong scheme here for the mirror-image reason.
The probe now uses https://biii.eu/, so an http listener or redirect problem can no longer be reported as an outage of a resource that is serving fine.
Every behaviour in this PR was verified with throwaway scripts that live
nowhere, which is a poor arrangement for the job whose whole purpose is to
tell us when the commons is broken. Twenty tests now live next to the script.
Covered: what counts as reachable (a 4xx is a status line, so the host is
serving; 5xx, the Cloudflare 52x codes and a refused connection are not), the
tcp:// probe, the rule that every target of a source must answer before a
failure is called ours, that a pull request sharing an issue's title is never
treated as the tracking issue, the issue list being fetched once, the full
issue lifecycle over six cases, and the three resilience properties: one
resource failing to report does not hide the others, a failure setting up the
label still produces a summary, and a run superseded by a newer one changes
nothing while the newest still acts.
Three of those cover regressions found during review, so they exist to keep
them found.
The suite is stdlib unittest, for the same reason the notifier is stdlib: the
thing that reports breakage should not depend on something that can break. It
is also fully offline -- every probe and every API call is stubbed -- so it is
fast and cannot fail because an upstream happens to be down. Exercising the
real probes is what DRY_RUN=1 on the notifier itself is for.
The Wiring tests are the ones likely to earn their keep longest: they assert
that the probe table, the jobs in import.yaml and the notify job's needs list
all agree. Renaming a source without updating the workflow fails the build,
which is checked by doing exactly that.
test-scripts.yml runs them on pull requests touching .github/scripts or
import.yaml. Tests nobody runs are decoration.
Also hardens superseded_by_newer_run() against a response that is not the
object it expects, which writing the harness turned up.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Running the new tests in .github/scripts leaves __pycache__ behind, and the
repository did not ignore it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The new job failed on its first run while the same tests passed locally, which
is exactly the kind of thing it was added to catch.
The suite used os.environ.setdefault for GITHUB_RUN_ID. On a runner that
variable is already set to the real run, so setdefault left it alone and the
tests ran against it: the stubbed "newest run" id of 100 never matched, every
run looked superseded, and the two tests that depend on a run acting failed.
Locally the variable is absent, the default applied, and everything passed.
The variables are now assigned rather than defaulted, DRY_RUN is cleared so an
ambient value cannot change the path under test, and GITHUB_STEP_SUMMARY is
removed so a stray call cannot append to the runner's real job summary.
Reproduced by running the suite with a real GITHUB_RUN_ID, a token and a
summary path in the environment: it fails before this change and passes after,
leaving the summary file untouched.
Also closes the workflow file the Wiring tests read, which the runner reported
as a ResourceWarning.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both review points are right, and the first is the same mistake twice.
The notifier lists this workflow's runs to decide whether a newer one has
started, which needs `actions: read`. The job-level permissions map named only
contents and issues, so that request would have been refused. The guard
catches its own error and carries on, so the effect was not a failure but a
silence: stale-run protection looked present and did nothing. This is the same
class of fault as the missing `contents: read` found earlier in this PR, in a
scope added after that one was fixed.
So rather than only adding the scope, there is now a test asserting that the
notify job grants every scope the script can reach, with the reason for each.
Dropping `actions: read` fails the suite, which is checked by doing exactly
that. A permissions bug that degrades silently at runtime should be a build
failure instead.
The fallback also now says what it is giving up -- "proceeding WITHOUT
stale-run protection" -- rather than reporting the error alone, since a 403
there is a configuration fault and not a blip.
test-scripts.yml's push filter did not include the workflow itself, so a
change touching only that file could reach master without the suite running,
even though the pull_request filter covered it. The two filters now match.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Prevent issue creation when outage label setup fails
.github/scripts/notify_upstream_failures.py:339
If ensure_label() fails (for example, a transient error while creating the new label), execution records the failure but continues into handle(). A subsequent issue create can then succeed with an unknown upstream-outage label, leaving an unlabelled tracking issue that find_issue() will never find on the next run, causing duplicates. Skip failure-side issue creation until the label is confirmed, while still emitting the summary and preserving recovery handling for already-labelled issues.
A hole in this PR's own mitigation, correctly spotted.
ensure_label() exists so the notifier does not depend on what GitHub does
with an unknown label at issue-creation time. When it failed, the run
recorded the failure and carried on into handle(), which created an issue
with labels: [upstream-outage] anyway -- depending on exactly that behaviour
again. If the label is dropped rather than created, the issue is unlabelled,
find_issue() filters on the label and never sees it, and a fresh issue is
opened every week: silent duplicates, which is the failure the label
machinery was added to prevent.
Issue creation is now held back until the label is confirmed. The run still
probes, still reports, still writes the summary and still exits non-zero; the
row says the issue was not opened and why. Recovery is unaffected, since
anything find_issue() can see was labelled when it was opened.
Two tests cover it: a failing label call opens nothing and says so, and a
recovery still closes an existing issue in the same conditions. Verified by
re-allowing creation, which fails the suite.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Correct — and it was a hole in this PR's own mitigation, which is the part worth naming.
ensure_label() exists so the notifier does not depend on what GitHub does with an unknown label at issue-creation time. When it failed, the run recorded the failure and then carried on into handle(), which created an issue with labels: [upstream-outage] anyway — depending on exactly that behaviour again. In the bad case the issue is unlabelled, find_issue() filters on the label and never sees it, and a fresh issue is opened every week. Silent duplicates: the precise failure the label machinery was added to prevent.
Issue creation is now held back until the label is confirmed. The run still probes, still reports, still writes the summary and still exits non-zero; the row records that the issue was not opened and why. Recovery is unaffected, since anything find_issue() can see was labelled when it was opened.
Two tests cover it — a failing label call opens nothing and says so, and a recovery still closes an existing issue under the same conditions — verified by re-allowing creation and watching the suite fail.
On the summary line for this review: it mentions "two moderate issues ... in resource tracking and issue creation, along with one wording correction", but only the issue-creation one is filed. I can't act on the other two as stated. If they can be pinned to a line I'll take a look.
The script was sitting in .github/scripts here, which is not where this
repository keeps that kind of code: every importer it reports on lives in
research-software-ecosystem/utils as a folder with an action.yml, called from
this workflow. The notifier now does the same, as
research-software-ecosystem/utils#68, and this PR is only the job that calls
it.
What moves with it: the script, its 27 tests and the workflow that runs them.
utils runs the suite on any change to that folder, so the tests did not go
anywhere, they just run next to the code they cover.
Two checks had to change shape, since the tests can no longer read this
workflow to compare it against the probe table:
- The table is now checked against `job-results` at runtime. A job listed
in `needs` with no upstream configured is reported as unprobed, and if it
failed the notifier exits non-zero, rather than a red import being
followed by a green notifier that says nothing about it.
- A missing `actions: read` here degrades the stale-run guard silently. The
action now says so in the job summary.
The checkout step is gone: the action brings its own script, so this job needs
no `contents` scope either.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
It was added for the test suite in .github/scripts, which now lives in
research-software-ecosystem/utils -- whose .gitignore already covers it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
You were right that this is where it belongs — every importer this job reports on is already a folder in utils with an action.yml, called from this workflow, and there was no reason for the notifier to be the exception. It is now notify-upstream-failures/, with the same shape: action.yml plus the script. No requirements.txt, because it is deliberately stdlib-only — the thing that tells you an import broke should not have a dependency that can break it — and I have said so in the manifest rather than adding an empty file and an install step.
This PR went from +821/−0 across four files to +45/−0 in import.yaml alone. The script, its 27 tests and the workflow that runs them all moved; utils runs the suite on any change to that folder. The __pycache__ entry I had added to .gitignore went too, since its reason went with the tests.
Two things did not survive the move unchanged, and they are the only part of this worth reviewing closely:
The probe table used to be checked against this file by a test. From utils it cannot be. That check now happens at runtime instead, against the needs this job passes in as job-results — which is better, since it is the real list in the real repository rather than a regex over YAML. If you add an importer here and to this job's needs but not to the action's table, the notifier reports it as not probed: no upstream configured and exits non-zero when it fails, instead of that importer failing unwatched.
A missing actions: read here silently degraded the stale-run guard. The check catches its own 403 and carries on, so nothing said the guard was gone. It now writes a warning into the job summary, and action.yml states the scopes the caller has to grant.
Also gone: the actions/checkout step and the contents: read scope. The action brings its own script, so this job checks nothing out.
Both sides verified before pushing — SOURCES and this job's needs agree exactly, and I ran the action's script against this workflow's real needs list to confirm the wiring produces the right outcome table.
utils#68 should merge first, since this pins @main.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a
notifyjob to the import workflow, so that a failed import says what broke instead of leaving a red square to be read through.The code it runs lives in research-software-ecosystem/utils#68, as
notify-upstream-failures/, packaged like the importer actions this workflow already calls. Merge that one first, since this pins to@main.Why
This run is the motivating case: bio.tools was down, the job went red, and telling that apart from a bug in the importer meant reading the log. Nobody is watching the log. The import is weekly, so a quiet failure can sit for a week or a month.
What the job does
For each importer that failed, the action re-probes that importer's upstream and reaches one of two verdicts:
upstream-unavailabledata/still holds the previous import and the next run resumes by itself.needs-attentionThe verdict is carried in one open issue per resource, labelled
upstream-outage. A new failure opens it, a change of verdict comments, an unchanged verdict stays silent so a long outage does not alert weekly, and a recovery comments and closes it. Every run writes an outcome table to the job summary either way, so a green run still shows what was checked.Notes on the job itself
if: always(). With the default, one failed importer would skip the notifier — precisely when it is wanted.permissions. A job-level map denies every scope it does not name.issues: writefor the tracking issue,actions: readfor the stale-run check. Nocontents: there is no checkout, because the action brings its own script.concurrency. find-then-create is not atomic across runners, so a manual dispatch overlapping the schedule could otherwise open duplicate issues for one outage.merge-imports, so it neither delays nor depends on the merge.History
The script first lived here in
.github/scripts. That is not where this repository keeps that kind of code — every importer this job reports on is a folder inutilswith anaction.yml— so it moved there, with its 27 tests and the workflow that runs them. What remains here is the call.One consequence worth flagging: the tests used to read this file and check the probe table against it, and from
utilsthey cannot. That check now happens at runtime instead, against theneedsthis job passes in. If you add an importer to this workflow and to this job'sneedswithout adding it to the action's table, the notifier reports it as unprobed and goes red when it fails — rather than an unwatched importer failing in silence.🤖 Generated with Claude Code