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

feat: Allow temp dir for poetry docker builds #638

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

gilandose
Copy link

@gilandose gilandose commented Nov 11, 2024

Description

Allows optionally specifying a temp dir for poetry to bring parity with npm and pip

Motivation and Context

Docker CLI on ubuntu default for github actions, does not allow mounting /tmp https://stackoverflow.com/questions/65267251/docker-bind-mount-directory-in-tmp-not-working
This results in a frustrating situation where terraform passes locally but fails with strange error in CI pipeline.

Fixes: #545

Breaking Changes

N/A

How Has This Been Tested?

  • I have updated at least one of the examples/* to demonstrate and validate my change(s)
  • I have tested and validated these changes using one or more of the provided examples/* projects
    Added temp dir to one poetry example and left rest untouched to prove not breaking change
  • I have executed pre-commit run -a on my pull request

@gilandose gilandose changed the title feat: allow temp dir for poetry docker builds feat: Allow temp dir for poetry docker builds Nov 11, 2024
@gilandose
Copy link
Author

@antonbabenko could I get a review on this when convenient, as this stops me from building layers in github action pipelines

@@ -426,6 +426,7 @@ module "lambda_layer_poetry" {
{
path = "${path.module}/../fixtures/python-app-poetry"
poetry_install = true
poetry_tmp_dir = "${path.root}/builds/"
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you change this line similar to the bellow existing in examples?

pip_tmp_dir = "${path.cwd}/../fixtures"

I think it's better to keep the builds/ folder with just the module generated content and doesn't give such advice in examples to use it for other purposes.

Copy link
Author

Choose a reason for hiding this comment

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

updated

@ahlinc
Copy link
Contributor

ahlinc commented Dec 1, 2024

@antonbabenko this change looks good to me and my latest changes that did to rework the build plan approach doesn't interfere with this one.

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.

Unable to use build_in_docker feature with Rancher Desktop or Colima
2 participants