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.

Sign up for GitHub

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 Dev Container configuration #8024

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

Add Dev Container configuration #8024

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
69431ef
add configuration + exclude devcontainer.json from hook
May 27, 2023
712ab6b
Remove blank line
May 27, 2023
98e917d
undo exclude
May 27, 2023
caf1e7a
formatting
May 27, 2023
6cb73a7
remove newline
May 27, 2023
1c43a32
remove auto open link as it does not support forks
May 27, 2023
e287fb9
reword
May 27, 2023
9cbb6b6
change context
May 27, 2023
844f788
format
May 27, 2023
f3e3d65
use python:3 for latest
May 28, 2023
0d8845b
Merge branch 'master' into savannah/devcontainer
Jun 14, 2023
9fc341f
Merge branch 'master' into savannah/devcontainer
Jun 27, 2023
7076013
Merge branch 'master' into savannah/devcontainer
Jul 4, 2023
ff80d37
Apply suggestions from code review
Jul 6, 2023
4372fdc
Merge branch 'master' into savannah/devcontainer
Jul 30, 2023
025ae58
Merge branch 'python-poetry:main' into savannah/devcontainer
savannahostrowski Apr 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .devcontainer/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
FROM mcr.microsoft.com/devcontainers/python:3-bookworm
Copy link
Member

@neersighted neersighted Feb 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Any chance we can use a standard library/python base image here? If not, what does the Microsoft-supplied base image add that enhances use of Dev Containers?

mcr.microsoft.com tends to be pretty slow for users in different parts of the world, compared to Docker Hub; though this is a secondary concern for me.


RUN export DEBIAN_FRONTEND=noninteractive \
&& apt-get update && apt-get install -y xdg-utils \
&& apt-get clean -y && rm -rf /var/lib/apt/lists/*
Comment on lines +3 to +5
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need xdg-utils? Does this add anything for users of a container used only via TTY?

Copy link
Member

Choose a reason for hiding this comment

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

It looks like this might relate to devcontainers/images#885 (not sure how VSCode wires that up -- maybe a script that emits an escape sequence?)... I'd rather let upstream figure this out (if we keep the Microsoft image) than try and solve it downstream.


ENV POETRY_HOME="/opt/poetry"
ENV PATH="$POETRY_HOME/bin:$PATH"
RUN curl -sSL https://install.python-poetry.org | python3 -
Comment on lines +3 to +9
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
RUN export DEBIAN_FRONTEND=noninteractive \
&& apt-get update && apt-get install -y xdg-utils \
&& apt-get clean -y && rm -rf /var/lib/apt/lists/*
ENV POETRY_HOME="/opt/poetry"
ENV PATH="$POETRY_HOME/bin:$PATH"
RUN curl -sSL https://install.python-poetry.org | python3 -
RUN pipx install poetry

Comment on lines +7 to +9
Copy link
Member

Choose a reason for hiding this comment

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

We don't really want to depend on the script here, and we're not actively encourage it's use either. Instead a manual install flow is perfectly suitable, in my opinion:

Suggested change
ENV POETRY_HOME="/opt/poetry"
ENV PATH="$POETRY_HOME/bin:$PATH"
RUN curl -sSL https://install.python-poetry.org | python3 -
ENV POETRY_HOME=/opt/poetry
RUN python3 -m venv $POETRY_HOME && \
$POETRY_HOME/bin/pip install poetry
ENV VIRTUAL_ENV=/opt/poetry-env
RUN python3 -m venv $VIRTUAL_ENV
ENV PATH="$VIRTUAL_ENV/bin:$POETRY_HOME/bin:$PATH"

This way we have a virtualenv holding Poetry (like the script creates, but explicitly controlled), and a separate virtual env for the project. This is a generic template following our recommendations that can be duplicated by other projects making use of Poetry + Dev Containers as well.

29 changes: 29 additions & 0 deletions .devcontainer/devcontainer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{
"name": "Poetry",
"build": {
"dockerfile": "Dockerfile",
"context": ".."
},
"postCreateCommand": "bash .devcontainer/setup.sh",
"customizations": {
"vscode": {
"settings": {
"python.formatting.blackPath": "black",
"python.formatting.provider": "black",
"python.testing.pytestEnabled": true,
"python.testing.pytestPath": "pytest",
"python.editor.codeActionsOnSave": {
"source.fixAll": true
},
"python.testing.pytestArgs": [
"tests"
]
},
"extensions": [
"ms-python.python",
"ms-python.black-formatter",
"charliermarsh.ruff"
]
}
}
}
4 changes: 4 additions & 0 deletions .devcontainer/setup.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
poetry install
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know the Devcontainer workflow. The poetry run pre-commit install confuse me. I don't know if the purpose of setup.sh is run before start working in the new feature/bug/etc, in that way the pre-commit install make sense for me, but the other commands don't.
Or when I finished to work in my new feature/bug/etc the setup.sh run, in that case I don't know if I want run the pre-commit install.
IMO it would be great write the new workflow (obviously if is necessary) in the documentation.

In any case, I like this idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, the setup.sh script runs just one time after the image has been created + the editor has launched (see postCreateCommand here).

The commands in this script are those which are denoted in https://python-poetry.org/docs/contributing/#local-development. You probably don't need to run poetry run mypy or poetry run pytest but I've included them to make sure everything is working as expected (doesn't really hurt to run them).

This script/configuration doesn't change anything about the core development workflow as prescribed in the docs today.

poetry run pytest
poetry run mypy
poetry run pre-commit install
Comment on lines +2 to +4
Copy link
Member

Choose a reason for hiding this comment

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

We should drop these lines; if the project is in a failing state we shouldn't cause issues with Dev Container creation... Imagine if a user modifies some files such that ruff is not happy, and then tries to activate Dev Containers with their editor -- now they have to stash or similar to avoid a failure here (and not all users will understand that).

8 changes: 8 additions & 0 deletions docs/contributing.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,14 @@ You should first fork the Poetry repository and then clone it locally, so that y
project. If you are new to Git and pull request based development, GitHub provides a
[guide](https://docs.github.com/en/get-started/quickstart/contributing-to-projects) you will find helpful.

{{% note %}}
The Poetry repo includes a [Dev Container](https://containers.dev/) configuration. By opening the repo in a Dev Container, you will have all tools/dependencies, as outlined by this guide, installed for you for easy contributing.

If you already have Visual Studio Code and Docker installed, you can install the [Dev Containers extension](https://marketplace.visualstudio.com/items?itemName=ms-vscode-remote.remote-containers) and then use the **Dev Containers: Clone Repository in Container Volume...** command to open your fork.

If you are using GitHub Codespaces, the environment will be automatically set up for you as the container is built.
{{% /note %}}

Next, you should install Poetry's dependencies, and run the test suite to make sure everything is working as expected:

```bash
Expand Down