chore: update sample script to accept positional argument for connector name #658
Conversation
📝 WalkthroughWalkthroughThe script Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Script
participant FileSystem
participant GSM
User->>Script: Run script with optional connector name argument
Script->>FileSystem: Check Airbyte repo root exists
alt Root missing
Script->>User: Raise FileNotFoundError
else Root exists
Script->>FileSystem: Ensure secrets directory exists (create if missing)
Script->>GSM: Fetch connector secrets (using connector name)
loop For each secret
Script->>FileSystem: Determine filename (from metadata or "config")
Script->>FileSystem: Write secret to JSON file in secrets directory
end
end
Would you like me to whip up a comparison diagram showing the old vs. new script flow for extra clarity, wdyt? Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 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 (6)
🪧 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
🧹 Nitpick comments (2)
examples/run_gsm_connector_secret_fetch.py (2)
39-42: Good directory validation and creationI like the safeguards you've added to check for directory existence. Creating the connector secrets directory if it doesn't exist (line 42) is a nice quality-of-life improvement that reduces the steps users need to take.
Since this is in the examples directory and per our guidelines we prefer brevity for demo scripts, would you consider simplifying the error message or combining these checks? Just a thought - the current implementation is perfectly fine too.
51-58: Well implemented secret fetching loop with flexible filename handlingThe improved loop now nicely handles dynamic connector names and provides sensible filename defaults. The addition of a default filename in line 54 is a good defensive programming practice.
One minor suggestion - would you consider adding a print statement to show which connector is being processed? Might be helpful for users to see what's happening, especially when running with default values. Something like:
def main() -> None: secret_mgr = GoogleGSMSecretManager( project=AIRBYTE_INTERNAL_GCP_PROJECT, credentials_json=ab.get_secret("GCP_GSM_CREDENTIALS"), ) + print(f"Fetching secrets for connector: {CONNECTOR_NAME}") secret: SecretHandleWhat do you think?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
examples/run_gsm_connector_secret_fetch.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`examples/*`: These scripts are intended for demonstration purposes only. They are not meant to represent production code. For these scripts, you should prefer brevity over robust ...
examples/*: These scripts are intended for demonstration purposes only. They are not meant to represent production code. For these scripts, you should prefer brevity over robust error handling and handling of edge cases. These are demos - which should be as simple as possible to handle the 'blessed' use cases.
examples/run_gsm_connector_secret_fetch.py
🧬 Code Graph Analysis (1)
examples/run_gsm_connector_secret_fetch.py (2)
airbyte/secrets/google_gsm.py (3)
GoogleGSMSecretManager(75-298)fetch_connector_secrets(243-262)labels(70-72)airbyte/secrets/base.py (2)
SecretHandle(207-263)write_to_file(240-263)
🪛 GitHub Actions: Run Linters
examples/run_gsm_connector_secret_fetch.py
[error] 1-1: Ruff formatting check failed. File would be reformatted. Run 'ruff format' to fix code style issues.
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: Pytest (All, Python 3.11, Windows)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Windows)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Pytest (No Creds)
- GitHub Check: Pytest (Fast)
🔇 Additional comments (4)
examples/run_gsm_connector_secret_fetch.py (4)
13-15: The updated usage instructions look great!Nice job updating the usage instructions to show the new flexibility. By providing several examples with different connector names, you've made it clear how users can take advantage of the new positional argument.
20-20: Clean import addition for command-line argument handlingThe sys import is necessary for accessing command line arguments. Good addition!
27-27: Effective implementation of the dynamic connector nameThis is a nice concise way to set the connector name from command line arguments with a sensible default. I notice you've changed the default from "source-s3" to "source-klaviyo" - was this intentional? Just checking in case it might affect existing usage, wdyt?
1-63: Please fix formatting issues reported by lintersThe pipeline is reporting that this file needs formatting with Ruff:
[error] 1-1: Ruff formatting check failed. File would be reformatted. Run 'ruff format' to fix code style issues.Could you run
ruff formaton this file to resolve the formatting issues?🧰 Tools
🪛 GitHub Actions: Run Linters
[error] 1-1: Ruff formatting check failed. File would be reformatted. Run 'ruff format' to fix code style issues.
|
/fix-pr
|
Modify the sample script to accept a connector name as a positional argument, allowing for more flexible usage. Update the script's usage instructions accordingly.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation