feat(cli): add support for building images for java connectors #522
Conversation
aaronsteers
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for building Docker images for Java connectors by introducing a new Dockerfile template and updating the image build and verification processes.
- Added a dedicated Java connector Dockerfile template.
- Updated the build process to run Gradle for creating the connector tar file.
- Improved JSON parsing in the image verification function to handle multi-line outputs.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| airbyte_cdk/utils/docker_image_templates.py | Introduces a Java connector Dockerfile template with custom settings. |
| airbyte_cdk/utils/docker.py | Updates _build_image to build the tar file for Java connectors and adjusts JSON parsing in verify_connector_image. |
📝 WalkthroughWalkthroughThe changes introduce support for Java connectors in the Docker build process within the Airbyte CDK. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BuildScript
participant Gradle
participant Docker
participant Connector
User->>BuildScript: Initiate Docker build for connector
BuildScript->>BuildScript: Check connector language
alt If Java connector
BuildScript->>Gradle: Run Gradle distTar for connector
Gradle-->>BuildScript: TAR file produced
BuildScript->>Docker: Build image using Java Dockerfile template
else Non-Java connector
BuildScript->>Docker: Build image using standard template
end
Docker-->>BuildScript: Image built
BuildScript->>Docker: Run connector image with spec command
Docker->>Connector: Execute spec
Connector-->>Docker: Output (possibly multi-line JSON)
Docker-->>BuildScript: Return output
BuildScript->>BuildScript: Parse each line for valid SPEC JSON
alt SPEC found
BuildScript-->>User: Verification successful
else
BuildScript-->>User: Verification failed
end
Suggested reviewers
Would you like to consider adding a test or example for the new Java Dockerfile template to help future maintainers, wdyt? 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (8)
✨ Finishing Touches
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. 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: 3
🧹 Nitpick comments (2)
airbyte_cdk/utils/docker_image_templates.py (2)
114-139: Hard-coded YUM commands may fail on future Corretto imagesThe new template assumes that
yumis available in theamazoncorretto:<version>image and uses the AL2023 packaging layout. While this is true today, the Corretto team occasionally rebases to newer AL versions and has switched package managers in the past (e.g. todnf/microdnf). If that happens every build will break.Would it be safer to:
- Invoke the package manager through the
/usr/bin/dnfsymlink which exists in bothyumanddnfimages, or- Delegate the installation to a slimmer “builder” stage and copy only the artefacts you need into the final image?
Happy to draft a small multi-stage alternative if you’d like—wdyt?
165-169: Single-stage image keeps build artefacts & increases sizeThe connector TAR is extracted in the same stage that becomes the runtime image, so the (possibly large) TAR layer is kept in the final history. A classic Docker optimisation is to:
- Run the
tar xf …in a builder stage,COPY --from=builder /airbyte /airbyteinto a slim runtime stage.This can shave off hundreds of MB for large connectors. Worth considering before this pattern gets duplicated—wdyt?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
airbyte_cdk/utils/docker.py(4 hunks)airbyte_cdk/utils/docker_image_templates.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
airbyte_cdk/utils/docker.py (2)
airbyte_cdk/sources/types.py (1)
data(35-36)airbyte_cdk/models/connector_metadata.py (2)
language(51-65)ConnectorLanguage(12-19)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- 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 (Fast)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
🔇 Additional comments (2)
airbyte_cdk/utils/docker.py (2)
69-69: Relative “three-levels-up” context is brittle
context_dir = context_dir.parent.parent.parentpresumes every Java connector lives exactly atairbyte-integrations/connectors/<name>. If that path ever changes the build will fail silently. Could we derive the repo root withgit rev-parse --show-toplevelor a pathlib search instead?
266-268: Nice! Java connectors now return the correct templateThe switch to return
JAVA_CONNECTOR_DOCKERFILE_TEMPLATElooks perfect and keeps the error handling consistent. 👍
There was a problem hiding this comment.
seems legit! I didn't review the dockerfile, but the python diff made sense. (I wouldn't be surprised if the dockerignore stuff causes some random weird connector to blow up, but we can deal that if/when it happens)
probably worth also tagging someone from db sources as fyi
This add the ability to build Java connector images.
This does not fully harden the image nor does it test every scenario needed to allow this to (yet) replace the
airbyte-cibuild and publish process. In future iterations, we would hopefully gain increasing confidence over time until we're ready to turn off the current Dagger-based processes.Combined with this PR, we'd be able to run FAST tests on any connector via docker, regardless of language:
Summary by CodeRabbit
Summary by CodeRabbit