-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[airbyte-ci] build python connectors from base images #30456
Merged
alafanechere
merged 27 commits into
master
from
09-14-change_our_python_connector_build_process_to_use_the_base_images
Oct 10, 2023
Merged
Changes from all commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
aa1d8cd
change our python connector build process to use the base images
alafanechere 36a0034
change our python connector build process to use the base images
alafanechere 992c318
revert changes on source-faker
alafanechere 64f2837
change interface to GLOBAL_REGISTRY
alafanechere 76c8fb0
do not interact with global registry on build
alafanechere ffec854
do not depend on base_images
alafanechere 9b5a698
port the development overrides to the new build process and test it
alafanechere 7284c23
do not pick connectors without metadata for testing
alafanechere 0419c95
mount airbyte-cdk during tests
alafanechere 8ba5cea
implement suggestions
alafanechere 30780d1
DEMO - to revert
alafanechere 249aed6
Revert "DEMO - to revert"
alafanechere 4590d20
Merge builder container and with_python_package to support poetry
bnchrch 77437b7
Remove Dagger Build hack
bnchrch 1817865
Add local cdk support
bnchrch 18693fc
Update default base image
bnchrch 2dbce7d
Automated Commit - Formatting Changes
bnchrch 04f32de
Move metadata check to get all connectors
bnchrch cd42288
remove explicit dev override mocks from test
bnchrch f600805
Merge branch 'master' into 09-14-change_our_python_connector_build_pr…
alafanechere 7db8c2d
make source-file-secure use the base image
alafanechere 3490668
revert source-file-secure changes
alafanechere 28dc026
rever source-file-secure changes
alafanechere 637103f
Automated Commit - Formatting Changes
alafanechere ca83df9
bump connector_ops version
alafanechere 9e07934
Merge branch '09-14-change_our_python_connector_build_process_to_use_…
alafanechere 5e1cacb
update poetry lock
alafanechere File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,7 @@ build-backend = "poetry.core.masonry.api" | |
|
||
[tool.poetry] | ||
name = "connector_ops" | ||
version = "0.2.4" | ||
version = "0.2.5" | ||
description = "Packaged maintained by the connector operations team to perform CI for connectors" | ||
authors = ["Airbyte <[email protected]>"] | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,8 +2,9 @@ | |
# Copyright (c) 2023 Airbyte, Inc., all rights reserved. | ||
# | ||
|
||
|
||
from dagger import Container, Platform | ||
from pipelines.actions.environments import with_airbyte_python_connector | ||
from pipelines.actions.environments import apply_python_development_overrides, with_python_connector_installed | ||
from pipelines.bases import StepResult | ||
from pipelines.builds.common import BuildConnectorImagesBase | ||
from pipelines.contexts import ConnectorContext | ||
|
@@ -15,8 +16,86 @@ class BuildConnectorImages(BuildConnectorImagesBase): | |
A spec command is run on the container to validate it was built successfully. | ||
""" | ||
|
||
async def _build_connector(self, platform: Platform) -> Container: | ||
return await with_airbyte_python_connector(self.context, platform) | ||
DEFAULT_ENTRYPOINT = ["python", "/airbyte/integration_code/main.py"] | ||
PATH_TO_INTEGRATION_CODE = "/airbyte/integration_code" | ||
|
||
async def _build_connector(self, platform: Platform): | ||
if ( | ||
"connectorBuildOptions" in self.context.connector.metadata | ||
and "baseImage" in self.context.connector.metadata["connectorBuildOptions"] | ||
): | ||
return await self._build_from_base_image(platform) | ||
else: | ||
return await self._build_from_dockerfile(platform) | ||
|
||
def _get_base_container(self, platform: Platform) -> Container: | ||
base_image_name = self.context.connector.metadata["connectorBuildOptions"]["baseImage"] | ||
self.logger.info(f"Building connector from base image {base_image_name}") | ||
return self.dagger_client.container(platform=platform).from_(base_image_name) | ||
|
||
async def _create_builder_container(self, base_container: Container) -> Container: | ||
"""Pre install the connector dependencies in a builder container. | ||
If a python connectors depends on another local python connector, we need to mount its source in the container | ||
This occurs for the source-file-secure connector for example, which depends on source-file | ||
|
||
Args: | ||
base_container (Container): The base container to use to build the connector. | ||
|
||
Returns: | ||
Container: The builder container, with installed dependencies. | ||
""" | ||
ONLY_PYTHON_BUILD_FILES = ["setup.py", "requirements.txt", "pyproject.toml", "poetry.lock"] | ||
builder = await with_python_connector_installed( | ||
self.context, | ||
base_container, | ||
str(self.context.connector.code_directory), | ||
include=ONLY_PYTHON_BUILD_FILES, | ||
) | ||
|
||
return builder | ||
|
||
async def _build_from_base_image(self, platform: Platform) -> Container: | ||
"""Build the connector container using the base image defined in the metadata, in the connectorBuildOptions.baseImage field. | ||
|
||
Returns: | ||
Container: The connector container built from the base image. | ||
""" | ||
self.logger.info("Building connector from base image in metadata") | ||
base = self._get_base_container(platform) | ||
builder = await self._create_builder_container(base) | ||
|
||
# The snake case name of the connector corresponds to the python package name of the connector | ||
# We want to mount it to the container under PATH_TO_INTEGRATION_CODE/connector_snake_case_name | ||
connector_snake_case_name = self.context.connector.technical_name.replace("-", "_") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📚 Let explain why we use snake case There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
|
||
connector_container = ( | ||
# copy python dependencies from builder to connector container | ||
base.with_directory("/usr/local", builder.directory("/usr/local")) | ||
.with_workdir(self.PATH_TO_INTEGRATION_CODE) | ||
.with_file("main.py", (await self.context.get_connector_dir(include="main.py")).file("main.py")) | ||
.with_directory( | ||
connector_snake_case_name, | ||
(await self.context.get_connector_dir(include=connector_snake_case_name)).directory(connector_snake_case_name), | ||
) | ||
.with_env_variable("AIRBYTE_ENTRYPOINT", " ".join(self.DEFAULT_ENTRYPOINT)) | ||
.with_entrypoint(self.DEFAULT_ENTRYPOINT) | ||
.with_label("io.airbyte.version", self.context.connector.metadata["dockerImageTag"]) | ||
.with_label("io.airbyte.name", self.context.connector.metadata["dockerRepository"]) | ||
) | ||
return connector_container | ||
|
||
async def _build_from_dockerfile(self, platform: Platform) -> Container: | ||
"""Build the connector container using its Dockerfile. | ||
|
||
Returns: | ||
Container: The connector container built from its Dockerfile. | ||
""" | ||
self.logger.warn( | ||
"This connector is built from its Dockerfile. This is now deprecated. Please set connectorBuildOptions.baseImage metadata field to use or new build process." | ||
) | ||
container = self.dagger_client.container(platform=platform).build(await self.context.get_connector_dir()) | ||
container = await apply_python_development_overrides(self.context, container) | ||
return container | ||
|
||
|
||
async def run_connector_build(context: ConnectorContext) -> StepResult: | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I think the list looks good generally for the cases we support, but is there any usecase for overriding these?
Also, do the dependencies on other local connectors still work with these whitelisted files? (assume so, but to confirm)
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.
I cant see a usecase for overriding these.
Basically this is defining "all ways to define python dependencies" and making sure they are mounted to the container.
If python adds yet another way, we would just extend this list instead of override.
also all the local dependency logic is still there and exists in
with_python_connector_installed