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

Add poetry to PATH #140

Merged
merged 1 commit into from
May 23, 2022

Conversation

TisVictress
Copy link
Contributor

This change will resolve #123

@TisVictress TisVictress requested a review from a team as a code owner May 23, 2022 17:47
@robdimsdale
Copy link
Member

This looks exactly like what I expected. and I can see that it works when I run the following:

pack build my-app --path ./integration/testdata/poetry
docker run -it my-app --entrypoint=launcher poetry --version
Poetry version 1.1.13

I think we can add an integration test here, under the existing test called: builds an oci image with poetry (dependency management only).

I hacked a test with the following:

			container, err = docker.Container.Run.
				WithTTY().
				WithEntrypoint("launcher").
				WithCommand("poetry --no-ansi --version"). // Use the no-ansi flag to disable color output - required for regex to match
				Execute(image.ID)
			Expect(err).NotTo(HaveOccurred())

			Eventually(func() string {
				cLogs, err := docker.Container.Logs.Execute(container.ID)
				Expect(err).NotTo(HaveOccurred())
				return cLogs.String()
			}).Should(MatchRegexp(`Poetry version \d+\.\d+\.\d+`))

But I haven't validated whether we can re-use the existing container - we might need to create a new container just to run this separate command.

@robdimsdale robdimsdale added the semver:minor A change requiring a minor version bump label May 23, 2022
@TisVictress TisVictress force-pushed the add-poetry-to-path branch from 353ebbd to 67a6512 Compare May 23, 2022 19:45
@TisVictress
Copy link
Contributor Author

@robdimsdale Thanks for the suggested changes! I've updated the PR.

Copy link
Member

@robdimsdale robdimsdale left a comment

Choose a reason for hiding this comment

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

LGTM - thanks for submitting this @TisVictress !

@robdimsdale robdimsdale merged commit 41faf08 into paketo-buildpacks:main May 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:minor A change requiring a minor version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Poetry to the PATH on the launch container
2 participants