-
Notifications
You must be signed in to change notification settings - Fork 17
tests: re-enable file-based connector (google-drive) in CDK downstream connector tests #498
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tests: re-enable file-based connector (google-drive) in CDK downstream connector tests #498
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR re-enables the file-based S3 connector in the CDK downstream connector tests. The key changes include uncommenting the S3 connector configuration and updating the workflow to run tests using the file-based connector.
📝 WalkthroughWalkthroughThis change updates the Changes
Possibly related PRs
Suggested labels
Would you like to consider any additional context or connectors for the test matrix, or does this cover your current needs—wdyt? 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 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 (1)
🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/connector-tests.yml (1)
78-78
: Comment on line 78 remains accurate: The note about destination‑pinecone being behind in CDK updates still applies to that connector. It’s fine to leave this comment as‑is, wdyt?
🧹 Nitpick comments (1)
airbyte_cdk/sources/file_based/discovery_policy/__init__.py (1)
9-10
: Remove placeholder comment: The# dummy change (revert me)
line appears to be a leftover placeholder without any functional impact. Consider removing it to keep the codebase clean, wdyt?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/connector-tests.yml
(1 hunks)airbyte_cdk/sources/file_based/discovery_policy/__init__.py
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Check: 'source-pokeapi' (skip=false)
- GitHub Check: Check: 'source-amplitude' (skip=false)
- GitHub Check: Check: 'source-s3' (skip=false)
- GitHub Check: Check: 'source-shopify' (skip=false)
- GitHub Check: Check: 'source-hardcoded-records' (skip=false)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (Fast)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
🔇 Additional comments (1)
.github/workflows/connector-tests.yml (1)
81-82
: Re-enable source-s3 connector: Great to seesource-s3
back in the matrix withcdk_extra: file-based
. Could you verify that thefile-based
filter in thecdk_changes
step correctly gates this job so it only runs when file-based code changes occur? wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
APPROVED
Thanks, @aldogonzalez8 ! 🙏 |
This PR re-enables testing for the
source-s3source-google-drive
connector in automated workflows. Previously this was commented out because it was too-far behind the latest version of the CDK. Now that the connector is caught up, this addition in the CDK will help us detect unexpected breakages.Test running here:
UPDATE: The above failed due to disk space. I don't know if that was an interim hiccup or something more serious. However it worked find for
source-google-drive
so I'm using that connector as suggested by @aldogonzalez8 in DM.Success log here:
I will only merge if the test completes successfully, and will revert my dummy commit if so.
Summary by CodeRabbit