From 6e181d16d281e75cce3d9f4f633318276a25c074 Mon Sep 17 00:00:00 2001 From: Roni Choudhury Date: Wed, 23 Oct 2024 10:07:22 -0400 Subject: [PATCH 01/12] Use pre-commit in tox configuration --- .pre-commit-config.yaml | 13 +++++++++++++ tox.ini | 17 +++++++---------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index a9ac085fb..76ebba42d 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -19,5 +19,18 @@ repos: hooks: # Run the linter. - id: ruff + # Run the formatter. - id: ruff-format + + # Run the linter in fix-only mode. + - id: ruff + alias: ruff-fix-only + stages: [manual] + args: [--fix-only] + + # Run the formatter in check mode. + - id: ruff-format + alias: ruff-format-check + stages: [manual] + args: [--check] diff --git a/tox.ini b/tox.ini index 26e310864..05b3906c6 100644 --- a/tox.ini +++ b/tox.ini @@ -11,22 +11,19 @@ envlist = package = skip ignore_errors = True deps = - codespell~=2.0 - ruff~=0.6.2 + pre-commit commands = - ruff --version - ruff check - ruff format --check - codespell . + pre-commit run --all-files ruff + pre-commit run --all-files --hook-stage manual ruff-format-check + pre-commit run --all-files codespell [testenv:format] package = skip deps = - ruff~=0.6.2 + pre-commit commands = - ruff --version - ruff check --fix-only - ruff format + pre-commit run --all-files --hook-stage manual ruff-fix-only + pre-commit run --all-files ruff-format [testenv:type] deps = From ffe0b1bf824ad0fb1334e6d6ab8a577309b6a67a Mon Sep 17 00:00:00 2001 From: Roni Choudhury Date: Fri, 1 Nov 2024 20:48:35 -0400 Subject: [PATCH 02/12] Build dev container as normal user --- dev/django.Dockerfile | 41 +++++++++++++++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/dev/django.Dockerfile b/dev/django.Dockerfile index 069a9105d..e21685167 100644 --- a/dev/django.Dockerfile +++ b/dev/django.Dockerfile @@ -1,4 +1,9 @@ FROM python:3.11-slim + +# Set some behaviors for Python. +ENV PYTHONDONTWRITEBYTECODE 1 +ENV PYTHONUNBUFFERED 1 + # Install system librarires for Python packages: # * psycopg2 RUN apt-get update && \ @@ -6,8 +11,21 @@ RUN apt-get update && \ libpq-dev gcc libc6-dev git && \ rm -rf /var/lib/apt/lists/* -ENV PYTHONDONTWRITEBYTECODE 1 -ENV PYTHONUNBUFFERED 1 +# Install pre-commit. Use `pip` instead of `apt` in order to get a recent +# version instead of whatever is in the image's package lists. +RUN pip install pre-commit + +# Create a normal user (that can be made to match the dev's user stats), falling +# back to root; if the root user is specified, don't actually run the `adduser` +# command. +ARG USERID=0 +ARG GROUPID=0 +ARG LOGIN=root +RUN if [ "${USERID}" != 0 ]; then adduser --uid ${USERID} --gid ${GROUPID} --home /home/${LOGIN} $LOGIN; fi + +# Create the project folder and make the user its owner. +RUN mkdir -p /opt/django-project +RUN chown ${USERID}:${GROUPID} /opt/django-project # Only copy the pyproject.toml, setup.py, and setup.cfg. It will still force all install_requires to be installed, # but find_packages() will find nothing (which is fine). When Docker Compose mounts the real source @@ -17,13 +35,28 @@ COPY ./pyproject.toml /opt/django-project/pyproject.toml COPY ./setup.cfg /opt/django-project/setup.cfg COPY ./setup.py /opt/django-project/setup.py -# Copy git folder for setuptools_scm -COPY ./.git/ /opt/django-project/.git/ +# Copy the pre-commit config so that the pre-commit environments can be +# constructed later. +COPY ./.pre-commit-config.yaml /opt/django-project/.pre-commit-config.yaml + +# Copy git folder for setuptools_scm. Make it owned by the user so that the +# pre-commit prep steps later don't complain about "dubious ownership" of the +# git repository. +COPY --chown=${USERID}:${GROUPID} ./.git/ /opt/django-project/.git/ # Don't install as editable, so that when the directory is mounted over with `docker compose`, the # installation still exists (otherwise the dandiapi.egg-info/ directory would be overwritten, and # the installation would no longer exist) RUN pip install /opt/django-project[dev] +# Switch to the normal user. +USER $LOGIN + # Use a directory name which will never be an import name, as isort considers this as first-party. WORKDIR /opt/django-project + +# Run the pre-commit hooks (without --all-files, so that it won't actually run +# on any files) to create a reusable package cache. +RUN pre-commit run ruff-fix-only --hook-stage manual +RUN pre-commit run ruff-format-check --hook-stage manual +RUN pre-commit run codespell From 3a9a3c6240756100c4812efa00b5360167ae9491 Mon Sep 17 00:00:00 2001 From: Roni Choudhury Date: Mon, 4 Nov 2024 14:28:05 -0500 Subject: [PATCH 03/12] Add documentation explaining how to build the container image properly --- DEVELOPMENT.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 3484b8c9d..ec7b25c8e 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -4,12 +4,13 @@ This is the simplest configuration for developers to start with. ### Initial Setup -1. Run `docker compose run --rm django ./manage.py migrate` -2. Run `docker compose run --rm django ./manage.py createcachetable` -3. Run `docker compose run --rm django ./manage.py createsuperuser --email $(git config user.email)` +1. Run `docker compose build --build-arg USERID=$(id -u) --build-arg GROUPID=$(id -g) --build-arg LOGIN=$(id -n -u)` to build the development container image. This builds the image to work with your (non-root) development user so that the linting and formatting commands work inside and outside of the container. If you prefer to build the container image so that it runs as `root`, you can omit the `--build-arg` arguments (but you will likely run into trouble running those commands). +2. Run `docker compose run --rm django ./manage.py migrate` +3. Run `docker compose run --rm django ./manage.py createcachetable` +4. Run `docker compose run --rm django ./manage.py createsuperuser --email $(git config user.email)` and follow the prompts to create your own user. This sets your username to your git email to ensure parity with how GitHub logins work. You can also replace the command substitution expression with a literal email address, or omit the `--email` option entirely to run the command in interactive mode. -4. Run `docker compose run --rm django ./manage.py create_dev_dandiset --owner $(git config user.email)` +5. Run `docker compose run --rm django ./manage.py create_dev_dandiset --owner $(git config user.email)` to create a dummy dandiset to start working with. ### Run Application @@ -21,7 +22,7 @@ This is the simplest configuration for developers to start with. Occasionally, new package dependencies or schema changes will necessitate maintenance. To non-destructively update your development stack at any time: 1. Run `docker compose pull` -2. Run `docker compose build --pull --no-cache` +2. Run `docker compose build --pull --no-cache --build-arg USERID=$(id -u) --build-arg GROUPID=$(id -g) --build-arg LOGIN=$(id -n -u)` (omitting the `--build-arg` arguments if you did so in Step 1 of *Initial Setup* above). 3. Run `docker compose run --rm django ./manage.py migrate` ## Develop Natively (advanced) From 6acf8163534fc1dc5adf6ec74487804d70d8e5de Mon Sep 17 00:00:00 2001 From: Roni Choudhury Date: Wed, 6 Nov 2024 11:34:45 -0500 Subject: [PATCH 04/12] Include instruction to run `docker compose pull` --- DEVELOPMENT.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index ec7b25c8e..cfc0cef36 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -4,13 +4,14 @@ This is the simplest configuration for developers to start with. ### Initial Setup -1. Run `docker compose build --build-arg USERID=$(id -u) --build-arg GROUPID=$(id -g) --build-arg LOGIN=$(id -n -u)` to build the development container image. This builds the image to work with your (non-root) development user so that the linting and formatting commands work inside and outside of the container. If you prefer to build the container image so that it runs as `root`, you can omit the `--build-arg` arguments (but you will likely run into trouble running those commands). -2. Run `docker compose run --rm django ./manage.py migrate` -3. Run `docker compose run --rm django ./manage.py createcachetable` -4. Run `docker compose run --rm django ./manage.py createsuperuser --email $(git config user.email)` +1. Run `docker compose pull` to ensure you have the latest versions of the service container images. +2. Run `docker compose build --build-arg USERID=$(id -u) --build-arg GROUPID=$(id -g) --build-arg LOGIN=$(id -n -u)` to build the development container image. This builds the image to work with your (non-root) development user so that the linting and formatting commands work inside and outside of the container. If you prefer to build the container image so that it runs as `root`, you can omit the `--build-arg` arguments (but you will likely run into trouble running those commands). +3. Run `docker compose run --rm django ./manage.py migrate` +4. Run `docker compose run --rm django ./manage.py createcachetable` +5. Run `docker compose run --rm django ./manage.py createsuperuser --email $(git config user.email)` and follow the prompts to create your own user. This sets your username to your git email to ensure parity with how GitHub logins work. You can also replace the command substitution expression with a literal email address, or omit the `--email` option entirely to run the command in interactive mode. -5. Run `docker compose run --rm django ./manage.py create_dev_dandiset --owner $(git config user.email)` +6. Run `docker compose run --rm django ./manage.py create_dev_dandiset --owner $(git config user.email)` to create a dummy dandiset to start working with. ### Run Application From 463109c835d13bfe6b34ae4136bbd62c3f012f1e Mon Sep 17 00:00:00 2001 From: Roni Choudhury Date: Mon, 11 Nov 2024 09:54:41 -0500 Subject: [PATCH 05/12] Revert move of `ENV` instructions --- dev/django.Dockerfile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dev/django.Dockerfile b/dev/django.Dockerfile index e21685167..0012ee34c 100644 --- a/dev/django.Dockerfile +++ b/dev/django.Dockerfile @@ -1,9 +1,5 @@ FROM python:3.11-slim -# Set some behaviors for Python. -ENV PYTHONDONTWRITEBYTECODE 1 -ENV PYTHONUNBUFFERED 1 - # Install system librarires for Python packages: # * psycopg2 RUN apt-get update && \ @@ -11,6 +7,10 @@ RUN apt-get update && \ libpq-dev gcc libc6-dev git && \ rm -rf /var/lib/apt/lists/* +# Set some behaviors for Python. +ENV PYTHONDONTWRITEBYTECODE 1 +ENV PYTHONUNBUFFERED 1 + # Install pre-commit. Use `pip` instead of `apt` in order to get a recent # version instead of whatever is in the image's package lists. RUN pip install pre-commit From 22c8fdf57a5264093b252760cd0ef518ea20c67f Mon Sep 17 00:00:00 2001 From: Roni Choudhury Date: Mon, 11 Nov 2024 11:51:58 -0500 Subject: [PATCH 06/12] Install pre-commit through setup.py --- dev/django.Dockerfile | 4 ---- setup.py | 1 + 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/dev/django.Dockerfile b/dev/django.Dockerfile index 0012ee34c..92eaaafcc 100644 --- a/dev/django.Dockerfile +++ b/dev/django.Dockerfile @@ -11,10 +11,6 @@ RUN apt-get update && \ ENV PYTHONDONTWRITEBYTECODE 1 ENV PYTHONUNBUFFERED 1 -# Install pre-commit. Use `pip` instead of `apt` in order to get a recent -# version instead of whatever is in the image's package lists. -RUN pip install pre-commit - # Create a normal user (that can be made to match the dev's user stats), falling # back to root; if the root user is specified, don't actually run the `adduser` # command. diff --git a/setup.py b/setup.py index 7c1023a96..0ad51c9f5 100644 --- a/setup.py +++ b/setup.py @@ -89,6 +89,7 @@ 'django-stubs', 'djangorestframework-stubs', 'types-setuptools', + 'pre-commit', ], 'test': [ 'factory-boy', From 2ada467972bb246e1a3a7faa6f3a21353d1e851f Mon Sep 17 00:00:00 2001 From: Roni Choudhury Date: Mon, 11 Nov 2024 11:54:16 -0500 Subject: [PATCH 07/12] Create the group if it doesn't already exist --- DEVELOPMENT.md | 2 +- dev/django.Dockerfile | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index cfc0cef36..5c8bffb16 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -5,7 +5,7 @@ This is the simplest configuration for developers to start with. ### Initial Setup 1. Run `docker compose pull` to ensure you have the latest versions of the service container images. -2. Run `docker compose build --build-arg USERID=$(id -u) --build-arg GROUPID=$(id -g) --build-arg LOGIN=$(id -n -u)` to build the development container image. This builds the image to work with your (non-root) development user so that the linting and formatting commands work inside and outside of the container. If you prefer to build the container image so that it runs as `root`, you can omit the `--build-arg` arguments (but you will likely run into trouble running those commands). +2. Run `docker compose build --build-arg USERID=$(id -u) --build-arg GROUPID=$(id -g) --build-arg LOGIN=$(id -n -u) --build-arg GROUPNAME=$(id -n -g)` to build the development container image. This builds the image to work with your (non-root) development user so that the linting and formatting commands work inside and outside of the container. If you prefer to build the container image so that it runs as `root`, you can omit the `--build-arg` arguments (but you will likely run into trouble running those commands). 3. Run `docker compose run --rm django ./manage.py migrate` 4. Run `docker compose run --rm django ./manage.py createcachetable` 5. Run `docker compose run --rm django ./manage.py createsuperuser --email $(git config user.email)` diff --git a/dev/django.Dockerfile b/dev/django.Dockerfile index 92eaaafcc..106340583 100644 --- a/dev/django.Dockerfile +++ b/dev/django.Dockerfile @@ -15,8 +15,10 @@ ENV PYTHONUNBUFFERED 1 # back to root; if the root user is specified, don't actually run the `adduser` # command. ARG USERID=0 -ARG GROUPID=0 ARG LOGIN=root +ARG GROUPID=0 +ARG GROUPNAME=root +RUN getent group ${GROUPID} || addgroup --gid ${GROUPID} ${GROUPNAME} RUN if [ "${USERID}" != 0 ]; then adduser --uid ${USERID} --gid ${GROUPID} --home /home/${LOGIN} $LOGIN; fi # Create the project folder and make the user its owner. From d7026cc83eb0e1f50062a2c976bcbb1cec7c27f0 Mon Sep 17 00:00:00 2001 From: Roni Choudhury Date: Mon, 11 Nov 2024 11:59:31 -0500 Subject: [PATCH 08/12] Allow usernames with `.` in them --- dev/django.Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/django.Dockerfile b/dev/django.Dockerfile index 106340583..0b895771a 100644 --- a/dev/django.Dockerfile +++ b/dev/django.Dockerfile @@ -19,7 +19,7 @@ ARG LOGIN=root ARG GROUPID=0 ARG GROUPNAME=root RUN getent group ${GROUPID} || addgroup --gid ${GROUPID} ${GROUPNAME} -RUN if [ "${USERID}" != 0 ]; then adduser --uid ${USERID} --gid ${GROUPID} --home /home/${LOGIN} $LOGIN; fi +RUN if [ "${USERID}" != 0 ]; then adduser --allow-bad-names --uid ${USERID} --gid ${GROUPID} --home /home/${LOGIN} $LOGIN; fi # Create the project folder and make the user its owner. RUN mkdir -p /opt/django-project From 3eeeeb9d8426f1c4eeef11a3bdd22d6f9c4e42a7 Mon Sep 17 00:00:00 2001 From: Roni Choudhury Date: Mon, 11 Nov 2024 12:00:22 -0500 Subject: [PATCH 09/12] Use `getent` to control conditional creation of user --- dev/django.Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/django.Dockerfile b/dev/django.Dockerfile index 0b895771a..be96216be 100644 --- a/dev/django.Dockerfile +++ b/dev/django.Dockerfile @@ -19,7 +19,7 @@ ARG LOGIN=root ARG GROUPID=0 ARG GROUPNAME=root RUN getent group ${GROUPID} || addgroup --gid ${GROUPID} ${GROUPNAME} -RUN if [ "${USERID}" != 0 ]; then adduser --allow-bad-names --uid ${USERID} --gid ${GROUPID} --home /home/${LOGIN} $LOGIN; fi +RUN getent passwd ${USERID} || adduser --allow-bad-names --uid ${USERID} --gid ${GROUPID} --home /home/${LOGIN} $LOGIN # Create the project folder and make the user its owner. RUN mkdir -p /opt/django-project From 026dc138dc581fb7805c48eddf299c55c25658a4 Mon Sep 17 00:00:00 2001 From: Roni Choudhury Date: Mon, 18 Nov 2024 13:53:41 -0500 Subject: [PATCH 10/12] Include GROUPNAME argument in second iteration of build command --- DEVELOPMENT.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index 5c8bffb16..f1c49c448 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -23,7 +23,7 @@ This is the simplest configuration for developers to start with. Occasionally, new package dependencies or schema changes will necessitate maintenance. To non-destructively update your development stack at any time: 1. Run `docker compose pull` -2. Run `docker compose build --pull --no-cache --build-arg USERID=$(id -u) --build-arg GROUPID=$(id -g) --build-arg LOGIN=$(id -n -u)` (omitting the `--build-arg` arguments if you did so in Step 1 of *Initial Setup* above). +2. Run `docker compose build --pull --no-cache --build-arg USERID=$(id -u) --build-arg GROUPID=$(id -g) --build-arg LOGIN=$(id -n -u) --build-arg GROUPNAME=$(id -n -g)` (omitting the `--build-arg` arguments if you did so in Step 1 of *Initial Setup* above). 3. Run `docker compose run --rm django ./manage.py migrate` ## Develop Natively (advanced) From 54c2dfd0ec1d862a6752d36e988ef42afd6afd1d Mon Sep 17 00:00:00 2001 From: Roni Choudhury <2903332+waxlamp@users.noreply.github.com> Date: Thu, 12 Dec 2024 12:42:36 -0500 Subject: [PATCH 11/12] Allow all group names Co-authored-by: Mike VanDenburgh <37340715+mvandenburgh@users.noreply.github.com> --- dev/django.Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev/django.Dockerfile b/dev/django.Dockerfile index be96216be..197a73eaa 100644 --- a/dev/django.Dockerfile +++ b/dev/django.Dockerfile @@ -18,7 +18,7 @@ ARG USERID=0 ARG LOGIN=root ARG GROUPID=0 ARG GROUPNAME=root -RUN getent group ${GROUPID} || addgroup --gid ${GROUPID} ${GROUPNAME} +RUN getent group ${GROUPID} || addgroup --allow-all-names --gid ${GROUPID} ${GROUPNAME} RUN getent passwd ${USERID} || adduser --allow-bad-names --uid ${USERID} --gid ${GROUPID} --home /home/${LOGIN} $LOGIN # Create the project folder and make the user its owner. From c7ce1ef745436690d1ed5e4438ed0332ed68e9d2 Mon Sep 17 00:00:00 2001 From: Roni Choudhury <2903332+waxlamp@users.noreply.github.com> Date: Fri, 13 Dec 2024 10:12:50 -0500 Subject: [PATCH 12/12] Simplify the `pre-commit` cache generation command Co-authored-by: Mike VanDenburgh <37340715+mvandenburgh@users.noreply.github.com> --- dev/django.Dockerfile | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/dev/django.Dockerfile b/dev/django.Dockerfile index 197a73eaa..841b673ec 100644 --- a/dev/django.Dockerfile +++ b/dev/django.Dockerfile @@ -55,6 +55,4 @@ WORKDIR /opt/django-project # Run the pre-commit hooks (without --all-files, so that it won't actually run # on any files) to create a reusable package cache. -RUN pre-commit run ruff-fix-only --hook-stage manual -RUN pre-commit run ruff-format-check --hook-stage manual -RUN pre-commit run codespell +RUN pre-commit