ci: fix downstream-basic trigger logic (#2474)#2489
Conversation
|
Hello, The CI workflows are currently pending manual approval. Could a maintainer please trigger them? I will mark the PR as ready for review once the tests pass. Thank you in advance. |
|
If this works as it appears, it should be a small improvement over the existing setup but not a full fix for #2474. I say "if" because the current CI will not confirm this until For liboqs/oqs-provider compatibility, I think we still need an explicit policy decision:
I would suggest that the policy decision be made first, then a full fix be proposed. |
xuganyu96
left a comment
There was a problem hiding this comment.
Hi @mohamadm0meni, thank you for your contribution!
The intention behind these jobs is to check that changes in the upstream project (i.e. liboq) do not break downstream projects (i.e. oqs-provider, openssh, boringssl, python/cpp/go/rust wrappers). Based on my reading, the problem with the current setup is that it only triggers the downstream projects' GitHub Actions workflows but do not check their results. Instead, the current checking logic only "intends to ensure" that the workflows have been triggered. Even that is not implemented correctly. For example, the URL for triggering oqs-provider's CI pipeline is simply wrong: we don't use Circle CI anymore and have moved everything onto GitHub Actions.
Please consider the following points when writing a complete solution:
- Do we want to interact with GitHub API (
https://api.github.com) or use some wrappers likeghCLI? (I personally prefer interacting with the HTTP API directly) - Do we want to isolate the logic into a separate script file living under
scripts/instead of writing inline bash scripts in workflow YAML? The former carries an additional advantage of making local runs easier. - How to check the status of the triggered workflow run? Check GitHub API's documentation: what are some terminal states? Also consider setting a timeout so this job doesn't run indefinitely. Given these requirements, we might even want to consider using a richer language like Python instead of bash scripting
References:
Signed-off-by: Mohamad Momeni <mohamadmomeni@eng.uk.ac.ir>
Signed-off-by: Mohamad Momeni <mohamadmomeni@eng.uk.ac.ir>
849f588 to
ffd8377
Compare
|
@xuganyu96 Thanks for the review. Updated to use the HTTP API directly via a stdlib-only Python script in scripts/, testable locally via --api-base. It now dispatches, locates the spawned run(s), and polls to a terminal status (with a 45-minute timeout), failing on anything other than success/neutral/skipped. CircleCI is removed entirely. |
Partially addresses #2474.
Replaces the CircleCI-based check for oqs-provider (decommissioned) and the substring/status-code check for the rest with a shared script, scripts/trigger_downstream_ci.py, that dispatches each downstream CI and polls the actual run to a terminal state before reporting success.
oqs-provider is temporarily excluded: none of its current workflows expose a repository_dispatch or workflow_dispatch trigger, so it cannot be wired in from this repo alone. A follow-up PR to oqs-provider is needed first.
Whether downstream compatibility should block liboqs PRs (vs. today's post-merge/manual checking) is a separate policy question, out of scope here.