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

Docs/docker best practices #9542

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

gianfa
Copy link

@gianfa gianfa commented Jul 10, 2024

Init Docker Best Practices documentation

Resolves: Discussion #1879

This PR is intended to begin the section on Docker Best Practices in Documentation.
It will serve as a collection point for all best practices (BPs) that will be recommended.
Hopefully, BPs, in order to be included and accepted here, will have to be accompanied by a concrete example that reproduces and demonstrates their effectiveness and by due references where possible.

The following contributions are proposed in this PR:

  • docs/docker-best-practices.md. Introduction file and list of best practices.
  • docker-examples/. Folder with practical examples to get you started. This already comes with two examples, also referenced in the docker-best-practices.md.

jstriebel and others added 7 commits July 24, 2020 15:06
This ensures to use the correct python executable.
Using `run` simply uses `sys.prefix` with a bin suffix, which is not necessarily the currently used python executable.

E.g. when linking a specific python version to a custom directory, using `sys.prefix` may point to the wrong python version:
* /usr/bin/python2.7
* /usr/bin/python3.6
* /usr/bin/python is a symlink to 2.7
* /usr/bin/local/python (or any other location) is a symlink to 3.6 which shadows the former symlink
`sys.prefix` is `/usr` in this case, which is correct, but the `python` executable there points to the wrong version.

`run_pip` in the env fixes this already by using `sys.executable`, see https://github.com/python-poetry/poetry/blob/develop/poetry/utils/env.py#L964
This should be used in the executor as well.
@gianfa gianfa marked this pull request as ready for review July 10, 2024 23:41
@radoering radoering force-pushed the docs/docker-best-practices branch from d12242c to 5500497 Compare October 12, 2024 13:58
@radoering radoering added the impact/docs Contains or requires documentation changes label Oct 12, 2024
Copy link

github-actions bot commented Oct 12, 2024

Deploy preview for website ready!

✅ Preview
https://website-mltjbl4g0-python-poetry.vercel.app

Built with commit 246e181.
This pull request is being automatically deployed with vercel-action

Copy link
Member

@radoering radoering left a comment

Choose a reason for hiding this comment

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

Some first feedback.

One general point: I am wondering why the project itself is not copied (after poetry install)? It feels like the examples stop halfway through.

docker-examples/minimum-poetry/Dockerfile Outdated Show resolved Hide resolved
docker-examples/minimum-poetry/Dockerfile Outdated Show resolved Hide resolved
docker-examples/poetry-multistage/Dockerfile Outdated Show resolved Hide resolved
@Secrus
Copy link
Member

Secrus commented Oct 18, 2024

I wonder if we could make a repo in our org to store those examples instead of putting them here. @radoering what do you think?

@radoering
Copy link
Member

I wonder if we could make a repo in our org to store those examples instead of putting them here. @radoering what do you think?

We could. Do you propose to only put the docker-examples directory into that repo and keep the docs here? This could be annoying because worst case you have to update two repos to update something. The other option would be to convert docs/docker-best-practices.md to the readme of the new repo and just put a link to the repo in the docs of poetry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/docs Contains or requires documentation changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants