feat: Migrate workflows to use resolve-vars-action@v0 #754
Conversation
- Replace manual gh api calls for PR info extraction - Replace hardcoded GitHub Actions run URL construction - Simplify fork detection logic in welcome message - Consolidate variable resolution across command workflows - Use standardized variable names from resolve-vars-action Updated workflows: - fix-pr-command.yml: Replace manual PR info extraction with resolve-vars-action - poetry-lock-command.yml: Replace manual PR info extraction with resolve-vars-action - test-pr-command.yml: Replace manual URL construction and simplify PR info handling - welcome-message.yml: Replace fork detection logic with resolve-vars-action Co-Authored-By: AJ Steers <[email protected]>
Original prompt from AJ Steers |
🤖 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:
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. Testing 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/1755464645-resolve-vars-action-migration' pyairbyte --help
# Install PyAirbyte from this branch for development:
pip install 'git+https://github.com/airbytehq/PyAirbyte.git@devin/1755464645-resolve-vars-action-migration'Helpful ResourcesPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
Community SupportQuestions? Join the #pyairbyte channel in our Slack workspace. |
There was a problem hiding this comment.
Pull Request Overview
This PR migrates PyAirbyte CI workflows to use a new resolve-vars-action@v0 for standardized variable resolution, eliminating manual GitHub API calls and reducing code duplication across slash command workflows.
- Replaces manual
gh apicalls andjqparsing with the resolve-vars-action in multiple workflows - Standardizes variable naming conventions with consistent
pr-source-*,pr-target-*, andresolved-*prefixes - Removes dependency on
cloudposse-github-actions/get-praction in favor of the new resolve-vars-action
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
.github/workflows/welcome-message.yml |
Replaces direct GitHub context fork detection with resolve-vars-action outputs |
.github/workflows/test-pr-command.yml |
Removes get-pr action dependency and manual URL construction, using resolve-vars-action instead |
.github/workflows/poetry-lock-command.yml |
Replaces manual GitHub API calls for PR info with resolve-vars-action |
.github/workflows/fix-pr-command.yml |
Replaces manual GitHub API calls for PR info with resolve-vars-action |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
The action was renamed from resolve-vars-action to resolve-ci-vars-action and republished under the new name. Co-Authored-By: AJ Steers <[email protected]>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughReplaces ad-hoc PR metadata retrieval across multiple GitHub Actions workflows with a centralized Resolve workflow variables step (aaronsteers/resolve-ci-vars-action@v0). Removes prior Get PR info steps, exposes standardized outputs (pr-source-*, pr-number, pr-title, run-url), and rewires downstream pushes, comments, and template inputs to use those outputs. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub
participant WF as Workflow Job
participant VARS as resolve-ci-vars Action
participant GIT as Git Remote
participant PR as Pull Request / Comment
GH->>WF: PR-triggered workflow start
WF->>VARS: Run Resolve workflow variables (aaronsteers/resolve-ci-vars-action)
VARS-->>WF: Outputs (pr-source-*, pr-number, pr-title, run-url)
WF->>GIT: Add remote & push to source repo/branch (using pr-source-*)
WF->>PR: Post comment with run-url and status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Remove log_outputs: true from resolve-ci-vars-action usage as it's not needed for the action to function correctly. Co-Authored-By: AJ Steers <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/welcome-message.yml (1)
41-41: Coerce fork flag to a boolean for robust template selectionAction outputs are strings; comparing to 'true' works but is brittle. Casting with fromJSON avoids surprises (e.g., accidental capitalization or truthy strings). Would you switch to boolean coercion for clarity and reliability, wdyt?
- template: ${{ steps.vars.outputs.pr-source-is-fork == 'true' && '.github/pr-welcome-community.md' || '.github/pr-welcome-internal.md' }} + template: ${{ fromJSON(steps.vars.outputs.pr-source-is-fork) && '.github/pr-welcome-community.md' || '.github/pr-welcome-internal.md' }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.github/workflows/fix-pr-command.yml(2 hunks).github/workflows/poetry-lock-command.yml(3 hunks).github/workflows/test-pr-command.yml(2 hunks).github/workflows/welcome-message.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Pytest (All, Python 3.11, Windows)
- GitHub Check: Pytest (All, Python 3.10, Windows)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Pytest (Fast)
- GitHub Check: Pytest (No Creds)
🔇 Additional comments (7)
.github/workflows/welcome-message.yml (2)
32-35: Good call centralizing PR metadata via resolve-ci-vars-actionAdopting the resolver step keeps this workflow lean and consistent with the others. LGTM, and the chosen outputs line up with the intended consumers, wdyt?
43-44: Template vars look accurate with new resolver outputsrepo_name and branch_name now correctly reference pr-source-* outputs. Nice alignment with the new naming scheme, wdyt?
.github/workflows/fix-pr-command.yml (1)
50-55: Nice: resolver step simplifies the workflow and exposes run-urlSwitching from ad-hoc gh/jq to resolve-ci-vars-action is a solid cleanup. Logging outputs here is helpful while we’re validating the first rollout, wdyt?
.github/workflows/test-pr-command.yml (2)
32-37: Resolver adoption looks correct and trims boilerplateGood swap to resolve-ci-vars-action with log_outputs enabled for traceability during rollout. This keeps the run URL and PR metadata standardized, wdyt?
51-56: Outputs mapping aligns with downstream usagesource-repo/branch/SHA/number/title/run-url point at the correct resolver outputs and match how they’re consumed later for checks and comments. Looks good, wdyt?
.github/workflows/poetry-lock-command.yml (2)
58-62: Consistent adoption of resolve-ci-vars-actionNice alignment with the other workflows and helpful to log outputs while validating. This removes the custom URL building and PR parsing nicely, wdyt?
126-126: No-op gating on a single diff looks correct for this workflowThere isn’t a second “unsafe” diff pass here, so relying solely on git-diff is the right simplification and resolves the earlier multi-diff gating concern, wdyt?
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
.github/workflows/test-pr-command.yml (1)
88-93: Checkout of fork PRs may fail without specifying the source repository.actions/checkout@v4 with only ref: defaults to the base repo (airbytehq/PyAirbyte). For forked PRs, that SHA won't exist in the base repo. Could we point checkout to the PR source repo/branch using the resolved vars, wdyt?
- - name: Checkout PR - uses: actions/checkout@v4 - with: - token: ${{ secrets.GITHUB_TOKEN }} - ref: ${{ needs.start-workflow.outputs.commit-sha }} + - name: Checkout PR + uses: actions/checkout@v4 + with: + # Public forks generally don't need auth to fetch, but keeping token reduces rate-limit issues. + token: ${{ secrets.GITHUB_TOKEN }} + repository: ${{ needs.start-workflow.outputs.source-repo }} + ref: ${{ needs.start-workflow.outputs.source-branch }}.github/workflows/poetry-lock-command.yml (1)
92-95: Use GITHUB_OUTPUT instead of deprecated ::set-output.::set-output is deprecated. Shall we switch to writing to $GITHUB_OUTPUT to avoid breakage and warnings, wdyt?
- run: | - git diff --quiet && echo "No changes to commit" || echo "::set-output name=changes::true" + run: | + if git diff --quiet; then + echo "No changes to commit" + else + echo "changes=true" >> "$GITHUB_OUTPUT" + fi
♻️ Duplicate comments (1)
.github/workflows/poetry-lock-command.yml (1)
107-112: Push to contributor fork is unauthenticated and non-idempotent.As-is, git push to https://github.com/.git will fail on forks without auth, and remote add may fail on reruns. Could we make it idempotent and embed a token (you already use GH_PAT_APPROVINGTON_OCTAVIA earlier), wdyt?
- - name: Push changes to '(${{ steps.vars.outputs.pr-source-repo-name-full }})' + - name: Push changes to '(${{ steps.vars.outputs.pr-source-repo-name-full }})' if: steps.git-diff.outputs.changes == 'true' run: | - git remote add contributor https://github.com/${{ steps.vars.outputs.pr-source-repo-name-full }}.git - git push contributor HEAD:${{ steps.vars.outputs.pr-source-git-branch }} + git remote remove contributor || true + git remote add contributor https://x-access-token:${{ secrets.GH_PAT_APPROVINGTON_OCTAVIA }}@github.com/${{ steps.vars.outputs.pr-source-repo-name-full }}.git + git push contributor HEAD:${{ steps.vars.outputs.pr-source-git-branch }}
🧹 Nitpick comments (2)
.github/workflows/test-pr-command.yml (1)
32-35: Adopt consistent action name and consider enabling debug logging (optional).You're using aaronsteers/resolve-ci-vars-action@v0, while the PR title/summary mentions resolve-vars-action@v0. Could you confirm the intended action name across workflows and standardize for consistency? Also, do you want to enable debug logs (log_outputs: true) here for parity and easier first-run troubleshooting, wdyt?
- - name: Resolve workflow variables - id: vars - uses: aaronsteers/resolve-ci-vars-action@v0 + - name: Resolve workflow variables + id: vars + uses: aaronsteers/resolve-ci-vars-action@v0 + with: + log_outputs: true.github/workflows/poetry-lock-command.yml (1)
58-61: Confirm action naming and (optionally) enable debug logs for first rollout.This uses aaronsteers/resolve-ci-vars-action@v0, while the PR summary mentions resolve-vars-action@v0. Could you standardize the name across workflows to avoid confusion? Also, would you like to enable log_outputs: true for initial observability, then turn it off later, wdyt?
- - name: Resolve workflow variables - id: vars - uses: aaronsteers/resolve-ci-vars-action@v0 + - name: Resolve workflow variables + id: vars + uses: aaronsteers/resolve-ci-vars-action@v0 + with: + log_outputs: true
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.github/workflows/fix-pr-command.yml(2 hunks).github/workflows/poetry-lock-command.yml(3 hunks).github/workflows/test-pr-command.yml(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/fix-pr-command.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Pytest (All, Python 3.10, Windows)
- GitHub Check: Pytest (All, Python 3.11, Windows)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (Fast)
- GitHub Check: Pytest (No Creds)
🔇 Additional comments (2)
.github/workflows/test-pr-command.yml (2)
63-71: Normalize OS labels to avoid runner label mismatch.runs-on composes "${{ matrix.os }}-latest". Using "Ubuntu" and "Windows" will produce "Ubuntu-latest"/"Windows-latest", which may not match runner labels. Shall we lower-case them to ensure "ubuntu-latest"/"windows-latest", wdyt?
- os: [ - Ubuntu, - Windows, - ] + os: [ + ubuntu, + windows, + ]
49-54: Nice move to standardized outputs.Switching to pr-source-* and run-url outputs tightens consistency and removes boilerplate. Looks good.
Co-authored-by: Copilot <[email protected]>
1 participant
feat: Migrate workflows to use resolve-vars-action@v0
Summary
This PR migrates PyAirbyte CI workflows to use the new
aaronsteers/resolve-vars-action@v0action, eliminating manual variable resolution patterns and reducing code duplication across slash command workflows.Key Changes:
gh apicalls +jqparsing with resolve-vars-actionpr-source-*,pr-target-*, andresolved-*prefixesThe resolve-vars-action automatically detects
prandcomment-idinputs from workflow_dispatch events and provides standardized outputs, eliminating ~20 lines of repetitive variable extraction code per workflow.Review & Testing Checklist for Human
steps.pr-info.outputs.repo→steps.vars.outputs.pr-source-repo-name-fullandsteps.pr-info.outputs.branch→steps.vars.outputs.pr-source-git-branchwork correctly/fix-pr,/test-pr, and/poetry-lockcommands on a test PR to ensure they work end-to-endsteps.vars.outputs.run-urlproduces correct GitHub Actions run URLs in comment linksprandcomment-idinputs are automatically detected and resolved by the actionRecommended Test Plan:
/fix-pr,/test-pr,/poetry-lock)prandcomment-idinputsDiagram
%%{ init : { "theme" : "default" }}%% graph TB subgraph "PyAirbyte Workflows" A["fix-pr-command.yml"]:::major-edit B["poetry-lock-command.yml"]:::major-edit C["test-pr-command.yml"]:::major-edit D["welcome-message.yml"]:::major-edit E["slash_command_dispatch.yml"]:::context end F["aaronsteers/resolve-vars-action@v0"]:::context A -->|"replaces gh api calls"| F B -->|"replaces gh api calls"| F C -->|"replaces manual URL construction"| F D -->|"replaces fork detection"| F E -->|"triggers workflows with pr/comment-id"| A E --> B E --> C subgraph Legend L1["Major Edit"]:::major-edit L2["Minor Edit"]:::minor-edit L3["Context/No Edit"]:::context end classDef major-edit fill:#90EE90 classDef minor-edit fill:#87CEEB classDef context fill:#FFFFFFNotes
github.event.pull_request.head.repo.fork == truetosteps.vars.outputs.pr-source-is-fork == 'true'(note the string comparison)Session Info:
Summary by CodeRabbit
Refactor
Chores
Important
Auto-merge enabled.
This PR is set to merge automatically when all requirements are met.
Note
Auto-merge may have been disabled. Please check the PR status to confirm.