Skip to content
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: only install main dependencies when calling poetry install #34945

Conversation

alafanechere
Copy link
Contributor

@alafanechere alafanechere commented Feb 7, 2024

What

We used the classic poetry install command in a container to install a package.
This commands also installs dev dependencies, which is not something we want when no additional depedencie groups are passed to the function

How

Run poetry install --only main when no additional_dependency_groups is passed to the _install_python_dependencies_from_poetry function.

Copy link

vercel bot commented Feb 7, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
airbyte-docs ⬜️ Ignored (Inspect) Visit Preview Feb 8, 2024 8:33am

Copy link
Contributor Author

alafanechere commented Feb 7, 2024

@alafanechere alafanechere force-pushed the augustin/02-07-airbyte-ci_only_install_main_dependencies_when_calling_poetry_install branch 2 times, most recently from 8e7457f to 8a0003a Compare February 7, 2024 10:03
@alafanechere alafanechere marked this pull request as ready for review February 7, 2024 10:03
@alafanechere alafanechere requested a review from a team as a code owner February 7, 2024 10:03
@alafanechere alafanechere force-pushed the augustin/02-07-airbyte-ci_only_install_main_dependencies_when_calling_poetry_install branch 3 times, most recently from 8a909b1 to 1b1e55b Compare February 7, 2024 16:36
@@ -159,12 +159,13 @@ def _install_python_dependencies_from_poetry(
) -> Container:
pip_install_poetry_cmd = ["pip", "install", "poetry"]
poetry_disable_virtual_env_cmd = ["poetry", "config", "virtualenvs.create", "false"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this imply that inside of the connector images, poetry will install deps into global python package space?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this allows us to then run the connector with the default entry point, as opposed to having to do poetry run whatever.py, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this imply that inside of the connector images, poetry will install deps into global python package space?

Yes, building the docker image is a two step approach:

  • We provision a builder container on which we run the package installation: this is where poetry is a requirement
  • We mount the installed packages to a new container which is transformed into our final image

This means that poetry could eventually not be bundled in our final images, the build dependencies being required only in the builder container.

But for simplicity the builder and the final connector image use the same base image: our python connector base image which has poetry.

If we'd want to optimize the image size we could just install build dependencies in the builder container.

@alafanechere alafanechere force-pushed the augustin/02-07-airbyte-ci_only_install_main_dependencies_when_calling_poetry_install branch from 1b1e55b to 05c08a5 Compare February 8, 2024 08:33
@alafanechere alafanechere merged commit dd112ee into master Feb 8, 2024
24 checks passed
@alafanechere alafanechere deleted the augustin/02-07-airbyte-ci_only_install_main_dependencies_when_calling_poetry_install branch February 8, 2024 08:55
jatinyadav-cc pushed a commit to ollionorg/datapipes-airbyte that referenced this pull request Feb 21, 2024
jatinyadav-cc pushed a commit to ollionorg/datapipes-airbyte that referenced this pull request Feb 26, 2024
jatinyadav-cc pushed a commit to ollionorg/datapipes-airbyte that referenced this pull request Feb 26, 2024
jatinyadav-cc pushed a commit to ollionorg/datapipes-airbyte that referenced this pull request Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants