Skip to content

Build: use same build environment for setup and build #9018

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

Merged
merged 1 commit into from
Mar 15, 2022

Conversation

humitos
Copy link
Member

@humitos humitos commented Mar 15, 2022

We can't do with self.build_environment: twice because the context manager
kills the Docker container on __exit__. So, the second time we call the with
it will create a new container which won't have all the work done in the
previous steps.

We could avoid this by using shared volumes instead, but that's a bigger
refactor. For now, I'm moving the part that creates the build environment back
to the Celery task and executing setup_environment and build steps from
there, which is not ideally, but solves the problem for now.

This problem was introduced in #9002

We can't do `with self.build_environment:` twice because the context manager
kills the Docker container on `__exit__`. So, the second time we call the `with`
it will create a new container which won't have all the work done in the
previous steps.

We could avoid this by using shared volumes instead, but that's a bigger
refactor. For now, I'm moving the part that creates the build environment back
to the Celery task and executing `setup_environment` _and_ `build` steps from
there, which is not ideally, but solves the problem for now.
@humitos humitos requested a review from a team as a code owner March 15, 2022 18:19
@ericholscher ericholscher merged commit 1825f28 into master Mar 15, 2022
@ericholscher ericholscher deleted the humitos/build-environment-bugfix branch March 15, 2022 22:41
@humitos humitos self-assigned this Mar 31, 2022
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.

2 participants