ci: run airbyte connector tests from master branch #99
Conversation
|
Warning Rate limit exceeded@aaronsteers has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 14 minutes and 43 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe pull request updates the GitHub Actions workflow for testing connectors. It retains existing triggers while suggesting future dependencies and clarifying comments in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
What do you think about these suggestions? wdyt? Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
.github/workflows/connector-tests.yml (3)
83-88: Add issue reference to the TODO comment?The comment explains why these connectors are disabled, but adding a reference to the GitHub issue tracking this would make it easier to follow up. WDYT about adding the issue URL mentioned in the PR description? 🤔
112-112: Consider making branch reference configurable?Instead of hardcoding
master, what do you think about using a variable like${{ github.event.pull_request.base.ref || 'master' }}? This would make it easier to test against different base branches when needed. 🤔
Line range hint
119-119: Add version or timeline to the TODO comment?The TODO mentions waiting for "Airbyte CI released" - would it be helpful to add the expected version number or approximate timeline? This could make it easier to track when to make this change. 🤔
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/connector-tests.yml(2 hunks)
🔇 Additional comments (2)
.github/workflows/connector-tests.yml (2)
90-90: LGTM: Clear job naming with skip status!
The dynamic name makes it super clear when jobs are skipped due to no relevant changes. Nice work! 👍
Line range hint 45-88: Verify connector matrix coverage
Let's make sure we haven't accidentally disabled any critical connectors.
✅ Verification successful
The previous command failed because we need to understand the repository structure first. Let's analyze the directory structure and then look for connectors using the CDK.
Matrix coverage looks good, but should we consider adding manifest connector tests? 🤔
The current matrix includes a good representation of different connector types:
- Basic CDK connectors (source-hardcoded-records, source-shopify, source-chargebee)
- File-based connector (source-s3)
- Vector DB connector (destination-pinecone)
- SQL connector (destination-motherduck)
The commented-out manifest connectors (source-the-guardian-api, source-pokeapi) are intentionally disabled with a clear TODO note about waiting for --use-local-cdk support. This is a known limitation that's being tracked.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check which connectors are currently using the CDK
cd airbyte
rg -l "airbyte-cdk" airbyte-integrations/connectors/*/metadata.yaml
Length of output: 280
Script:
#!/bin/bash
# Let's check the repository structure first
fd --type directory "connectors$" --max-depth 3
# Then check for any metadata.yaml files
fd "metadata.yaml$"
# Let's also check the matrix in the workflow file directly
cat .github/workflows/connector-tests.yml
Length of output: 6600
| @@ -116,8 +117,7 @@ jobs: | |||
| if: steps.no_changes.outputs.status != 'cancelled' | |||
| with: | |||
| repository: airbytehq/airbyte | |||
| # TODO: Revert to `master` after Airbyte CI released: | |||
| ref: aj/airbyte-ci/update-python-local-cdk-code | |||
| ref: master | |||
This reverts a previous change that pulled airbyte-ci and connector images from a fixed branch where we were iterating on airbyte-ci improvements.
After this change:
masterbranch on the monorepo.--use-local-cdkor similar) #64Summary by CodeRabbit