feat: add airbyte-cdk CLI with support for: connector test, secrets fetch and --version
#493
Conversation
…d for classes starting with "Test"
![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
🧹 Nitpick comments (3)
docs/CONTRIBUTING.md (1)
58-61: Minor grammar tweak for smoother readingThe phrase “Include an inline comment explaining why the ignore is needed.” sounds slightly awkward because “ignore” is being used as a noun. Would you consider re‑phrasing to “Include an inline comment explaining why the ignore rule is needed.” wdyt?
🧰 Tools
🪛 LanguageTool
[grammar] ~60-~60: The verb ‘ignore’ does not usually follow articles like ‘the’. Check that ‘ignore’ is spelled correctly; using ‘ignore’ as a noun may be non-standard.
Context: ...nclude an inline comment explaining why the ignore is needed. Example: ```toml [tool.dep...(A_INFINITIVE)
airbyte_cdk/cli/airbyte_cdk/__init__.py (1)
55-60: Leverageclick.version_optionfor a simpler--versionflag
clickalready provides a convenient decorator (@click.version_option) that handles the--versionflag (including-V, eager evaluation, etc.). Using it would remove the manual flag wiring and a handful of lines. Something like:-@click.option( - "--version", - is_flag=True, - help="Show the version of the Airbyte CDK.", -) +@click.version_option(message="Airbyte CDK version: %(version)s")The decorator could call
print_versioninternally (or you can drop that indirection entirely). What do you think?airbyte_cdk/cli/airbyte_cdk/_secrets.py (1)
105-113: Support default Google credentials for smoother UX?Right now the command errors if
GCP_GSM_CREDENTIALSis absent. Many GCP environments rely on the default credentials file (GOOGLE_APPLICATION_CREDENTIALS) or on workload identity. Would it be useful to fall back tosecretmanager.SecretManagerServiceClient()without explicit credentials whenGCP_GSM_CREDENTIALSisn’t set, and only raise if that also fails?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
airbyte_cdk/cli/airbyte_cdk/__init__.py(1 hunks)airbyte_cdk/cli/airbyte_cdk/_secrets.py(1 hunks)docs/CONTRIBUTING.md(1 hunks)pyproject.toml(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
🧰 Additional context used
🧬 Code Graph Analysis (1)
airbyte_cdk/cli/airbyte_cdk/__init__.py (5)
airbyte_cdk/cli/airbyte_cdk/_connector.py (1)
connector_cli_group(92-94)airbyte_cdk/cli/airbyte_cdk/_image.py (1)
image_cli_group(8-10)airbyte_cdk/cli/airbyte_cdk/_manifest.py (1)
manifest_cli_group(14-18)airbyte_cdk/cli/airbyte_cdk/_secrets.py (1)
secrets_cli_group(35-37)airbyte_cdk/cli/airbyte_cdk/_version.py (1)
print_version(7-13)
🪛 LanguageTool
docs/CONTRIBUTING.md
[grammar] ~60-~60: The verb ‘ignore’ does not usually follow articles like ‘the’. Check that ‘ignore’ is spelled correctly; using ‘ignore’ as a noun may be non-standard.
Context: ...nclude an inline comment explaining why the ignore is needed. Example: ```toml [tool.dep...
(A_INFINITIVE)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Check: 'source-pokeapi' (skip=false)
- GitHub Check: Check: 'source-amplitude' (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 (All, Python 3.10, Ubuntu)
- GitHub Check: preview_docs
- GitHub Check: Pytest (Fast)
- GitHub Check: SDM Docker Image Build
- GitHub Check: Analyze (python)
🔇 Additional comments (1)
airbyte_cdk/cli/airbyte_cdk/__init__.py (1)
70-76:❌ Incorrect review comment
Sub‑command groups may not be valid
click.Commandobjects
cli.add_command(connector_cli_group)(and the three siblings) assumes that each imported symbol is already aclick.Command/Group. The snippet in_connector.pyshows a plain function stub returningNone, which would cause a runtime failure:TypeError: Command is not a click command (got None)Could you double‑check that the actual implementation decorates those callables with
@click.group()(or returns a properly configured command) before registering them?
🏁 Script executed:
#!/bin/bash # Locate where connector_cli_group is defined or imported rg -n 'connector_cli_group' -n . echo # List all definitions of any *_cli_group to see if they’re decorated rg -n 'def .*_cli_group' -n . echo # Search for @click.group decorators within the CLI package rg -n '@click\.group' -n airbyte_cdk/cli/airbyte_cdkLength of output: 1202
No Issue: CLI Groups Are Already Decorated
I’ve confirmed that each*_cli_groupfunction is decorated with@click.group, so they return validclick.Groupobjects and can be safely registered viacli.add_command. wdyt?• airbyte_cdk/cli/airbyte_cdk/_connector.py:91 –
@click.group(name="connector")
• airbyte_cdk/cli/airbyte_cdk/_manifest.py:13 –@click.group(name="manifest")
• airbyte_cdk/cli/airbyte_cdk/_image.py:7 –@click.group(name="image")
• airbyte_cdk/cli/airbyte_cdk/_secrets.py:31 –@click.groupLikely an incorrect or invalid review comment.
|
/autofix
|
|
/poetry-lock
|
|
/poetry-lock
|
Overview
This PR makes the CDK a true "developer kit" and not just a library.
Simple functions can be performed directly from the new
airbyte-cdkCLI, starting with:airbyte-cdk --help- Print usage info and exit.airbyte-cdk --version- Print the CDK version and exit.airbyte-cdk connector test- Test the connector using the new FAST Airbyte Standard Tests framework. (No stub file required, and connector type is autodetected frommetadata.yaml.)airbyte-cdk secrets fetch- Fetch the secrets for a given connector into the connector'ssecretsdirectory (e.g.source-s3/secretsfor the S3 source.(Each command group and command also support
--help.)CLI Screenshot:
--helpLoom Demo
https://www.loom.com/share/970666990f564eed8dda8e089cfbe6df?sid=798c51df-73a5-44d6-b3be-6e9c64872931
Future Work (Not in Scope Here)
In a future iteration, I'm planning to add
--docker-imagesupport withinconnector testcommand. This would run all of the FAST Standard tests against the docker image, rather than using the local python environment. (This is mostly useful in CI to ensure the image is healthy, or for Java/Kotlin connectors.)I also added stubs for
manifestandimagecommand groups, which I expect we may want to populate in the future with something like...:airbyte-cdk image build(in progress here: feat(cli): Addimage buildcommand toairbyte-cdkCLI #489)airbyte-cdk manifest validateairbyte-cdk manifest test-readHow many CLI entrypoints would we have after this?
I think the goal is to have exactly 2 CLI entrypoints:
source-declarative-manifest- this is literally howsource-declarative-manifestis invoked as a connector and it lives in the CDK.airbyte-cdk- This is the new "dev tools" CLI - and we can put all manner of dev-related tools in here as needed. This should replace the more fragile and awkwardpython -m ....-based invocations. By structuring under command groups (secrets,manifest,connector, etc.), we can avoid needing multiple entrypoints for different purposes.How is this invoked?
There are three invocation patterns:
pipx run airbyte-cdk ...oruvx airbyte-cdk .... This doesn't require a pre-install and it uses the latest version by default.poetry run airbyte-cdk .... This uses exactly the version of the CDK that you have in your dev environment, and it runs directly within your venv. When you bump the CDK version in your project, you also bump the CDK CLI version.pipx install airbyte-cdkoruv tool install airbyte-cdkallows you to just invoke directly with no prefix:airbyte-cdk .... If you use this method, you just have to ensure you remember to upgrade occassionally.Why choose
airbyte-cdkas the name of the CLI entrypoint?There are really two main reasons I arrived on using the
airbyte-cdkname:First, the name aligns exactly with the functions it delivers: these are the "connector developer kit" actions that are needed by developers during development.
Second, by aligning the name of the CLI with the name of the package, we get a much more concise stateless invocation when running with
pipxoruvx.When names don't match, you end up having to type both the package name and the CLI name like this (using
--fromor--spec):But because the CLI name matches the package name, we can use this much more concise shorthand, where the package name and CLI name are one and the same:
Summary by CodeRabbit
.tmpto.gitignoreto exclude temporary files.