ci: migrate workflow-dispatch to benc-uk/workflow-dispatch v1.3.0#986
ci: migrate workflow-dispatch to benc-uk/workflow-dispatch v1.3.0#986Aaron ("AJ") Steers (aaronsteers) wants to merge 4 commits intomainfrom
Conversation
Migrate from the-actions-org/workflow-dispatch@v4 to benc-uk/workflow-dispatch@v1.3.0 which uses the new GitHub API return_run_details parameter for deterministic run ID resolution. Changes: - Pin to benc-uk/workflow-dispatch@a54f9d1 (v1.3.0) - wait-for-completion-timeout -> wait-timeout-seconds (1800s = 30m) - workflow-url output -> runUrlHtml output - Remove workflow-conclusion output (not available in benc-uk) Co-Authored-By: AJ Steers <aj@airbyte.io>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksTesting This PyAirbyte VersionYou can test this version of PyAirbyte using the following: # Run PyAirbyte CLI from this branch:
uvx --from 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1771663959-migrate-workflow-dispatch' pyairbyte --help
# Install PyAirbyte from this branch for development:
pip install 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1771663959-migrate-workflow-dispatch'PR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
📚 Show Repo GuidanceHelpful ResourcesCommunity SupportQuestions? Join the #pyairbyte channel in our Slack workspace. |
📝 WalkthroughWalkthroughSwaps the workflow-dispatch action implementation, renames the completion timeout parameter to seconds, and updates public outputs: removes Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Did you verify that the new action exposes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
benc-uk/workflow-dispatch v1.3.0 does not fail the step when the dispatched workflow fails (it only checks status, not conclusion). Add an explicit gh run view check to fail the job properly when the dispatched workflow concludes with anything other than success. Co-Authored-By: AJ Steers <aj@airbyte.io>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/prerelease-command.yml (1)
93-102: Nice — this directly addresses the failure propagation gap from the previous review.The
gh run view+ conclusion check is the right approach given that benc-uk v1.3.0 doesn't fail the step on a non-success dispatched run. A couple of small observations:
Hardcoded repo: Line 95 uses
--repo airbytehq/PyAirbyte. Would--repo ${{ github.repository }}be slightly more portable here, wdyt? It's the same value in practice, but avoids the coupling if the workflow ever gets forked or the repo gets renamed.Timeout edge case: If the 1800s wait expires (benc-uk logs a warning but doesn't fail), this step would still run and
conclusionwould likely benull/empty since the run is still in-progress. The!= "success"check handles that correctly — the job will fail. Just wanted to confirm that's the intended behavior.Optional: use
github.repositoryinstead of hardcoded repo- name: Verify dispatched workflow succeeded run: | - CONCLUSION=$(gh run view ${{ steps.dispatch.outputs.runId }} --repo airbytehq/PyAirbyte --json conclusion -q .conclusion) + CONCLUSION=$(gh run view ${{ steps.dispatch.outputs.runId }} --repo ${{ github.repository }} --json conclusion -q .conclusion) echo "Dispatched workflow conclusion: $CONCLUSION" if [ "$CONCLUSION" != "success" ]; then echo "::error::Dispatched workflow concluded with: $CONCLUSION" exit 1 fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/prerelease-command.yml around lines 93 - 102, Replace the hardcoded repo in the gh command so the step uses the current repository dynamically: update the gh invocation that uses "gh run view ${{ steps.dispatch.outputs.runId }} --repo airbytehq/PyAirbyte" to use --repo ${{ github.repository }} (keeping the same runId reference steps.dispatch.outputs.runId and GH_TOKEN env) to avoid coupling to a specific repo; leave the existing conclusion check logic as-is to treat null/empty conclusions (e.g., from timeout) as failures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/prerelease-command.yml:
- Around line 93-102: Replace the hardcoded repo in the gh command so the step
uses the current repository dynamically: update the gh invocation that uses "gh
run view ${{ steps.dispatch.outputs.runId }} --repo airbytehq/PyAirbyte" to use
--repo ${{ github.repository }} (keeping the same runId reference
steps.dispatch.outputs.runId and GH_TOKEN env) to avoid coupling to a specific
repo; leave the existing conclusion check logic as-is to treat null/empty
conclusions (e.g., from timeout) as failures.
Co-Authored-By: AJ Steers <aj@airbyte.io>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/prerelease-command.yml (1)
93-102: Nice addition for catching silent dispatch failures! One optional hardening: move the inline expressions to env vars.GitHub's security hardening guide recommends that for inline scripts, the preferred approach is to set expression values to intermediate environment variables, rather than interpolating
${{ }}expressions directly into the shell script. WhilerunId(a numeric value) andgithub.repositorycarry near-zero injection risk here, applying the env-var pattern consistently is good hygiene, wdyt?🔒 Proposed hardening
- name: Verify dispatched workflow succeeded run: | - CONCLUSION=$(gh run view ${{ steps.dispatch.outputs.runId }} --repo ${{ github.repository }} --json conclusion -q .conclusion) + CONCLUSION=$(gh run view "$RUN_ID" --repo "$REPO" --json conclusion -q .conclusion) echo "Dispatched workflow conclusion: $CONCLUSION" if [ "$CONCLUSION" != "success" ]; then echo "::error::Dispatched workflow concluded with: $CONCLUSION" exit 1 fi env: GH_TOKEN: ${{ steps.get-app-token.outputs.token }} + RUN_ID: ${{ steps.dispatch.outputs.runId }} + REPO: ${{ github.repository }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/prerelease-command.yml around lines 93 - 102, Move inline GitHub Actions expressions into environment variables and reference those vars in the shell block: define env entries for the dispatched run id (using steps.dispatch.outputs.runId) and the repository (using github.repository) and reuse the existing GH_TOKEN (steps.get-app-token.outputs.token), then update the script to call gh run view using those env vars and assign the result to CONCLUSION as before; ensure CONCLUSION, the echoed message, and the error/exit logic remain unchanged but reference the new env variable names instead of interpolated expressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/prerelease-command.yml:
- Around line 93-102: Move inline GitHub Actions expressions into environment
variables and reference those vars in the shell block: define env entries for
the dispatched run id (using steps.dispatch.outputs.runId) and the repository
(using github.repository) and reuse the existing GH_TOKEN
(steps.get-app-token.outputs.token), then update the script to call gh run view
using those env vars and assign the result to CONCLUSION as before; ensure
CONCLUSION, the echoed message, and the error/exit logic remain unchanged but
reference the new env variable names instead of interpolated expressions.
|
Re: CodeRabbit's suggestion to move |
Co-Authored-By: AJ Steers <aj@airbyte.io>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/prerelease-command.yml:
- Around line 91-94: The dispatch step currently uses wait-for-completion and
wait-timeout-seconds but does not verify the triggered workflow's conclusion,
allowing silent success when the publish fails or times out; add a follow-up
verification step after the dispatch (referencing the dispatch step outputs like
steps.dispatch.outputs.runId or runUrlHtml) that runs gh run view --json
conclusion (or uses the prior action that checks conclusion) and explicitly
fails the job when the returned conclusion is not "success" so that
needs.build-and-publish.result reflects real failure and the failure comment
path executes.
1 participant
ci: migrate workflow-dispatch to benc-uk/workflow-dispatch v1.3.0
Summary
Migrates the prerelease workflow from
the-actions-org/workflow-dispatch@v4(a fork) to the upstreambenc-uk/workflow-dispatch@v1.3.0, which now uses GitHub's newreturn_run_detailsAPI parameter for deterministic run ID resolution instead of pollinglistWorkflowRuns. See benc-uk/workflow-dispatch#83.Changes:
the-actions-org/workflow-dispatch@v4→benc-uk/workflow-dispatch@a54f9d1(v1.3.0, SHA-pinned)wait-for-completion-timeout: 30m→wait-timeout-seconds: 1800workflow-url→runUrlHtmlworkflow-conclusion(not available in benc-uk; replaced by explicitgh run viewconclusion check — see below)gh run viewwith therunIdoutput to check the dispatched run'sconclusionand fails the job on non-success. This is necessary because benc-uk v1.3.0'swait-for-completiononly checksstatus(completed vs. in-progress), notconclusion(success vs. failure) — it logs a warning instead of failing the step.Review & Testing Checklist for Human
/prereleaseslash command) and verify: (a) the workflow dispatches successfully, (b) the status comment posts with a valid workflow URL, (c) a deliberately failing publish still produces the failure comment. This is the only way to validate these changes — CI cannot exercise the dispatch path.gh run viewverification step is untested. Confirm it correctly reads therunIdoutput from the dispatch step and that the conclusion value is"success"for a passing run. Edge case: if the 1800s wait timeout expires, the run may still be in-progress —conclusionwould benull/empty, which the!= "success"check handles by failing, but verify this is the desired behavior.steps.dispatch.outputs.runUrlHtmlproduces a valid URL in the PR success/failure comments.Notes
oncallrepo already usesbenc-uk/workflow-dispatch@v1, and the floatingv1tag now points to v1.3.0 — no changes needed there.benc-uk/workflow-dispatchv1.3.0 was merged 9 hours ago — it's very fresh. Worth monitoring for any upstream issues.Requested by: Aaron ("AJ") Steers (@aaronsteers)
Link to Devin run
Summary by CodeRabbit