[airbyte-ci] implement pre/post build hooks #30526

alafanechere · 2023-09-18T14:16:40Z

What

Closes #30237
In some scenario a connector might have to customize our default build process: to install a system dependency, a system env var.
To do so we want to expose a pre/post build hook mechanism that connector developers can leverage.

How

Developers will create a build_customization.py module in the connector code directory
They'll have to implement a pre_connector_install and/or post_connector_install function. They'll mutate the container object of this stage in this function.
Our build process will run this function: pre_connector_install is executed on top of our base image, post_connector_install is executed after the rest of all the build instructions.

alafanechere · 2023-09-18T14:16:53Z

Current dependencies on/for this PR:

master
- PR [airbyte-ci] build python connectors from base images #30456

This comment was auto-generated by Graphite.

github-actions · 2023-09-18T14:17:02Z

Before Merging a Connector Pull Request

Wow! What a great pull request you have here! 🎉

To merge this PR, ensure the following has been done/considered for each connector added or updated:

PR name follows PR naming conventions
Breaking changes are considered. If a Breaking Change is being introduced, ensure an Airbyte engineer has created a Breaking Change Plan.
Connector version has been incremented in the Dockerfile and metadata.yaml according to our Semantic Versioning for Connectors guidelines
You've updated the connector's metadata.yaml file any other relevant changes, including a breakingChanges entry for major version bumps. See metadata.yaml docs
Secrets in the connector's spec are annotated with airbyte_secret
All documentation files are up to date. (README.md, bootstrap.md, docs.md, etc...)
Changelog updated in docs/integrations/<source or destination>/<name>.md with an entry for the new version. See changelog example
Migration guide updated in docs/integrations/<source or destination>/<name>-migrations.md with an entry for the new version, if the version is a breaking change. See migration guide example
If set, you've ensured the icon is present in the platform-internal repo. (Docs)

If the checklist is complete, but the CI check is failing,

Check for hidden checklists in your PR description
Toggle the github label checklist-action-run on/off to re-run the checklist CI.

bnchrch · 2023-09-18T17:56:09Z

airbyte-ci/connectors/pipelines/pipelines/builds/python_connectors.py

    PATH_TO_INTEGRATION_CODE = "/airbyte/integration_code"

+    @staticmethod
+    def get_main_file_name(build_customization_module: Optional[ModuleType]) -> str:


💅 A docstring here would be lovely!

bnchrch · 2023-09-18T17:58:04Z

airbyte-ci/connectors/pipelines/pipelines/builds/python_connectors.py

@@ -76,29 +89,58 @@ async def _provision_builder_container(self, base_container: Container) -> Conta

        return builder.with_exec(["pip", "install", "--prefix=/install", "."])

+    def _get_build_customization_module(self) -> Optional[ModuleType]:


❓ We will be doing this for java connectors correct? If so lets move this to a more generic location

bnchrch · 2023-09-18T18:06:03Z

airbyte-ci/connectors/pipelines/pipelines/builds/python_connectors.py

    async def _build_from_base_image(self) -> 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.
        """
        base = self._get_base_container()
+        build_customization_module = self._get_build_customization_module()


Im thinking hooks related functions like _get_build_customization_module, and calls to pre_connector_install should move to a folder/file named something like build_hooks or build_lifecycle

e.g.

# pipelines/builds/hooks/_init__.py def get_build_customization_module(connector: Connector) -> Optional[ModuleType]: # ... def pre_install_hook(connector: Connector, container: Container) -> Container build_customization_module = get_build_customization_module(connector) if hasattr(build_customization_module, "pre_connector_install"): self.logger.info("Adding the pre_connector_install hook to the base") container = await build_customization_module.pre_connector_install(container) return container def post_install_hook(connector: Connector, container: Container) -> Container build_customization_module = get_build_customization_module(connector) if hasattr(build_customization_module, "post_connector_install"): self.logger.info("Adding the post_connector_install hook to the base") container = await build_customization_module.post_connector_install(container) return container

It would also give us a focused spot to document the connector lifecycle in a README.md file

what hooks are available?

what do they do?

how do you use them?

Thanks for the suggestion, will do.

erohmensing

Nice work! I agree with ben about pulling the build lifecycle into a separate place so its visible, great suggestion. No blocking comments from me :)

erohmensing · 2023-09-21T14:14:02Z

airbyte-ci/connectors/pipelines/pipelines/builds/python_connectors.py

    PATH_TO_INTEGRATION_CODE = "/airbyte/integration_code"

+    @staticmethod
+    def get_main_file_name(build_customization_module: Optional[ModuleType]) -> str:
+        if build_customization_module is not None and hasattr(build_customization_module, "MAIN_FILE_NAME"):


Do we have any validation that an existing build_customization.py will define this constant? if not we might end up with a None here

erohmensing · 2023-09-21T14:18:30Z

airbyte-ci/connectors/pipelines/tests/test_builds/test_python_connectors.py

@@ -31,9 +33,17 @@ def test_context_with_connector_without_base_image(self, test_context):
    def connector_with_base_image(self, all_connectors):


Suggested change

def connector_with_base_image(self, all_connectors):

def connector_with_base_image_no_customization(self, all_connectors):

or something of the sort?

Or it might be worth having both of these as separate fixtures. E.g. if I add something to the base image and want to assert that all connectors (regardless of customization) have it, I might want this. If I am changing some behavior that build customization might affect, I want to get the list of non-customized connectors?

erohmensing · 2023-09-21T14:21:08Z

airbyte-ci/connectors/pipelines/tests/test_builds/test_python_connectors.py

        pytest.skip("No connector with a connectorBuildOptions.baseImage metadata found")

+    @pytest.fixture
+    def connector_with_base_image_with_build_customization(self, connector_with_base_image):


❓ seeing that our other fixture uses real connectors, in the future would we potentially change this to return all (real) connectors with build customizations?

Yes why not! It will slow down the test but add coverage.
In any case I think we shall make an automation that will require a review from our team if a connector is implementing a build_customization module.

That sounds like a good idea! At least for the foreseeable future, while we work out any kinks and there are few examples to work off of

erohmensing · 2023-09-21T14:21:22Z

airbyte-ci/connectors/pipelines/tests/test_builds/test_python_connectors.py

+        dummy_build_customization = (Path(__file__).parent / "dummy_build_customization.py").read_text()
+        (connector_with_base_image.code_directory / "build_customization.py").write_text(dummy_build_customization)
+        yield connector_with_base_image
+        (connector_with_base_image.code_directory / "build_customization.py").unlink()


Just curious. what does unlink() do?

It removes the file. Basically a rm

…zation.py

airbyte-oss-build-runner · 2023-10-11T13:36:15Z

source-zendesk-chat test report (commit `fcecb62e5b`) - ✅

⏲️ Total pipeline duration: 02mn00s

Step	Result
Build source-zendesk-chat docker image for platform(s) linux/x86_64	✅
Unit tests	✅
Acceptance tests	✅
Code format checks	✅
Validate metadata for source-zendesk-chat	✅
Connector version semver check	✅
Connector version increment check	✅
QA checks	✅

🔗 View the logs here

☁️ View runs for commit in Dagger Cloud

Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command

airbyte-ci connectors --name=source-zendesk-chat test

This reverts commit 950ff92.

alafanechere force-pushed the augustin/09-18-_airbyte-ci_Implement_pre/post_build_hooks branch from db634ba to 8e3804a Compare September 18, 2023 14:16

octavia-squidington-iii added the area/connectors Connector related issues label Sep 18, 2023

octavia-squidington-iii added the connectors/source/zendesk-chat label Sep 18, 2023

alafanechere mentioned this pull request Sep 18, 2023

[airbyte-ci] new commands for migration to base image #30520

Merged

alafanechere force-pushed the 09-14-change_our_python_connector_build_process_to_use_the_base_images branch from 73377ba to 25f394d Compare September 18, 2023 14:45

alafanechere force-pushed the augustin/09-18-_airbyte-ci_Implement_pre/post_build_hooks branch from 8e3804a to 82971ce Compare September 18, 2023 14:46

This was referenced Sep 18, 2023

[airbyte-ci]: new check on python certified connector to validate their base image use #30527

Merged

Make certified python connectors use our base image #30528

Closed

alafanechere changed the title ~~[airbyte-ci] Implement pre/post build hooks~~ [airbyte-ci] implement pre/post build hooks Sep 18, 2023

alafanechere marked this pull request as ready for review September 18, 2023 15:21

octavia-squidington-iii removed the area/connectors Connector related issues label Sep 18, 2023

airbytehq deleted a comment from github-actions bot Sep 18, 2023

alafanechere requested a review from a team September 18, 2023 16:13

bnchrch reviewed Sep 18, 2023

View reviewed changes

alafanechere force-pushed the 09-14-change_our_python_connector_build_process_to_use_the_base_images branch from 25f394d to 5cc21b3 Compare September 19, 2023 15:28

alafanechere force-pushed the augustin/09-18-_airbyte-ci_Implement_pre/post_build_hooks branch from f09dfd8 to 0ec7047 Compare September 19, 2023 15:28

alafanechere force-pushed the augustin/09-18-_airbyte-ci_Implement_pre/post_build_hooks branch 2 times, most recently from 276f1b0 to 404346d Compare September 21, 2023 11:05

erohmensing approved these changes Sep 21, 2023

View reviewed changes

bnchrch force-pushed the 09-14-change_our_python_connector_build_process_to_use_the_base_images branch from 96068a5 to 18693fc Compare September 28, 2023 15:16

Base automatically changed from 09-14-change_our_python_connector_build_process_to_use_the_base_images to master October 10, 2023 17:16

alafanechere force-pushed the augustin/09-18-_airbyte-ci_Implement_pre/post_build_hooks branch 2 times, most recently from 27bc360 to 62fe3cb Compare October 11, 2023 12:59

octavia-squidington-iii added area/connectors Connector related issues connectors/source/auth0 labels Oct 11, 2023

alafanechere force-pushed the augustin/09-18-_airbyte-ci_Implement_pre/post_build_hooks branch from 62fe3cb to a728afd Compare October 11, 2023 13:01

octavia-squidington-iii removed the area/connectors Connector related issues label Oct 11, 2023

alafanechere added 2 commits October 11, 2023 15:22

[airbyte-ci] Implement pre/post build hooks

987bb9e

remove dockerfile for source-zendesk-chat and implement build_customi…

fcecb62

…zation.py

alafanechere force-pushed the augustin/09-18-_airbyte-ci_Implement_pre/post_build_hooks branch from a728afd to fcecb62 Compare October 11, 2023 13:32

octavia-squidington-iii added area/connectors Connector related issues area/documentation Improvements or additions to documentation labels Oct 11, 2023

vercel bot deployed to Preview October 11, 2023 13:33 View deployment

alafanechere added the checklist-action-run label Oct 11, 2023

alafanechere merged commit 950ff92 into master Oct 11, 2023
27 of 31 checks passed

alafanechere deleted the augustin/09-18-_airbyte-ci_Implement_pre/post_build_hooks branch October 11, 2023 13:53

alafanechere added a commit that referenced this pull request Oct 11, 2023

Revert "[airbyte-ci] implement pre/post build hooks (#30526)"

5b806a8

This reverts commit 950ff92.

alafanechere mentioned this pull request Oct 11, 2023

Revert "[airbyte-ci] implement pre/post build hooks" #31280

Merged

alafanechere restored the augustin/09-18-_airbyte-ci_Implement_pre/post_build_hooks branch October 11, 2023 14:07

alafanechere added a commit that referenced this pull request Oct 11, 2023

[airbyte-ci] implement pre/post build hooks (#30526)

a3f900b

alafanechere mentioned this pull request Oct 11, 2023

[airbyte-ci] Implement pre/post build hooks #31282

Merged

edgao pushed a commit that referenced this pull request Oct 11, 2023

[airbyte-ci] implement pre/post build hooks (#30526)

35d34da

alafanechere added a commit that referenced this pull request Oct 12, 2023

[airbyte-ci] implement pre/post build hooks (#30526)

5d7a8c3

ariesgun pushed a commit to ariesgun/airbyte that referenced this pull request Oct 20, 2023

[airbyte-ci] implement pre/post build hooks (airbytehq#30526)

cf7bc84

ariesgun pushed a commit to ariesgun/airbyte that referenced this pull request Oct 23, 2023

[airbyte-ci] implement pre/post build hooks (airbytehq#30526)

72360cd

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[airbyte-ci] implement pre/post build hooks #30526

[airbyte-ci] implement pre/post build hooks #30526

alafanechere commented Sep 18, 2023 •

edited

Loading

alafanechere commented Sep 18, 2023 •

edited by bnchrch

Loading

github-actions bot commented Sep 18, 2023 •

edited by alafanechere

Loading

bnchrch Sep 18, 2023

bnchrch Sep 18, 2023

bnchrch Sep 18, 2023 •

edited

Loading

alafanechere Sep 18, 2023

erohmensing left a comment

erohmensing Sep 21, 2023

erohmensing Sep 21, 2023

erohmensing Sep 21, 2023

erohmensing Sep 21, 2023

alafanechere Sep 21, 2023

erohmensing Sep 21, 2023

erohmensing Sep 21, 2023

alafanechere Sep 21, 2023

airbyte-oss-build-runner commented Oct 11, 2023

		@@ -76,29 +89,58 @@ async def _provision_builder_container(self, base_container: Container) -> Conta

		return builder.with_exec(["pip", "install", "--prefix=/install", "."])

		def _get_build_customization_module(self) -> Optional[ModuleType]:

		@@ -31,9 +33,17 @@ def test_context_with_connector_without_base_image(self, test_context):
		def connector_with_base_image(self, all_connectors):

	def connector_with_base_image(self, all_connectors):
	def connector_with_base_image_no_customization(self, all_connectors):

[airbyte-ci] implement pre/post build hooks #30526

[airbyte-ci] implement pre/post build hooks #30526

Conversation

alafanechere commented Sep 18, 2023 • edited Loading

What

How

alafanechere commented Sep 18, 2023 • edited by bnchrch Loading

github-actions bot commented Sep 18, 2023 • edited by alafanechere Loading

Before Merging a Connector Pull Request

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bnchrch Sep 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erohmensing left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

airbyte-oss-build-runner commented Oct 11, 2023

source-zendesk-chat test report (commit fcecb62e5b) - ✅

alafanechere commented Sep 18, 2023 •

edited

Loading

alafanechere commented Sep 18, 2023 •

edited by bnchrch

Loading

github-actions bot commented Sep 18, 2023 •

edited by alafanechere

Loading

bnchrch Sep 18, 2023 •

edited

Loading

source-zendesk-chat test report (commit `fcecb62e5b`) - ✅