Improve handling common parser arguments (#2221)

.github/workflows/docker-build-test-upload.yml

@@ -66,8 +66,13 @@ jobs:
         shell: bash
 
       - name: Write tags file 🏷
-        run: |
+        run: >
-          python3 -m tagging.write_tags_file --short-image-name ${{ inputs.image }} --tags-dir /tmp/jupyter/tags/ --registry ${{ env.REGISTRY }} --owner ${{ env.OWNER }} --variant ${{ inputs.variant }}
+          python3 -m tagging.write_tags_file
+          --registry ${{ env.REGISTRY }}
+          --owner ${{ env.OWNER }}
+          --short-image-name ${{ inputs.image }}
+          --variant ${{ inputs.variant }}
+          --tags-dir /tmp/jupyter/tags/
         shell: bash
       - name: Upload tags file 💾
         uses: actions/upload-artifact@v4
@@ -77,7 +82,14 @@ jobs:
           retention-days: 3
 
       - name: Write manifest and build history file 🏷
-        run: python3 -m tagging.write_manifest --short-image-name ${{ inputs.image }} --hist-lines-dir /tmp/jupyter/hist_lines/ --manifests-dir /tmp/jupyter/manifests/ --registry ${{ env.REGISTRY }} --owner ${{ env.OWNER }} --variant ${{ inputs.variant }}
+        run: >
+          python3 -m tagging.write_manifest
+          --registry ${{ env.REGISTRY }}
+          --owner ${{ env.OWNER }}
+          --short-image-name ${{ inputs.image }}
+          --variant ${{ inputs.variant }}
+          --hist-lines-dir /tmp/jupyter/hist_lines/
+          --manifests-dir /tmp/jupyter/manifests/
         shell: bash
       - name: Upload manifest file 💾
         uses: actions/upload-artifact@v4
@@ -105,5 +117,9 @@ jobs:
           retention-days: 3
 
       - name: Run tests ✅
-        run: python3 -m tests.run_tests --short-image-name ${{ inputs.image }} --registry ${{ env.REGISTRY }} --owner ${{ env.OWNER }}
+        run: >
+          python3 -m tests.run_tests
+          --registry ${{ env.REGISTRY }}
+          --owner ${{ env.OWNER }}
+          --short-image-name ${{ inputs.image }}
         shell: bash

.github/workflows/docker-merge-tags.yml

@@ -62,5 +62,9 @@ jobs:
 
       - name: Merge tags for the images 🔀
         if: env.PUSH_TO_REGISTRY == 'true'
-        run: python3 -m tagging.merge_tags --short-image-name ${{ inputs.image }} --tags-dir /tmp/jupyter/tags/ --variant ${{ inputs.variant }}
+        run: >
+          python3 -m tagging.merge_tags
+          --short-image-name ${{ inputs.image }}
+          --variant ${{ inputs.variant }}
+          --tags-dir /tmp/jupyter/tags/
         shell: bash

.github/workflows/docker-tag-push.yml

@@ -56,7 +56,14 @@ jobs:
           name: ${{ inputs.image }}-${{ inputs.platform }}-${{ inputs.variant }}-tags
           path: /tmp/jupyter/tags/
       - name: Apply tags to the loaded image 🏷
-        run: python3 -m tagging.apply_tags --short-image-name ${{ inputs.image }} --tags-dir /tmp/jupyter/tags/ --platform ${{ inputs.platform }} --registry ${{ env.REGISTRY }} --owner ${{ env.OWNER }} --variant ${{ inputs.variant }}
+        run: >
+          python3 -m tagging.apply_tags
+          --registry ${{ env.REGISTRY }}
+          --owner ${{ env.OWNER }}
+          --short-image-name ${{ inputs.image }}
+          --variant ${{ inputs.variant }}
+          --platform ${{ inputs.platform }}
+          --tags-dir /tmp/jupyter/tags/
       # This step is needed to prevent pushing non-multi-arch "latest" tag
       - name: Remove the "latest" tag from the image 🗑️
         run: docker image rmi ${{ env.REGISTRY }}/${{ env.OWNER }}/${{ inputs.image }}:latest

.github/workflows/docker-wiki-update.yml

@@ -39,7 +39,11 @@ jobs:
           path: wiki/
 
       - name: Update wiki 🏷
-        run: python3 -m tagging.update_wiki --wiki-dir wiki/ --hist-lines-dir /tmp/jupyter/hist_lines/ --manifests-dir /tmp/jupyter/manifests/
+        run: >
+          python3 -m tagging.update_wiki
+          --wiki-dir wiki/
+          --hist-lines-dir /tmp/jupyter/hist_lines/
+          --manifests-dir /tmp/jupyter/manifests/
         shell: bash
 
       - name: Push Wiki to GitHub 📤

Makefile

@@ -79,25 +79,25 @@ linkcheck-docs: ## check broken links
 hook/%: VARIANT?=default
 hook/%: ## run post-build hooks for an image
 	python3 -m tagging.write_tags_file \
-	  --short-image-name "$(notdir $@)" \
-	  --tags-dir /tmp/jupyter/tags/ \
 	  --registry "$(REGISTRY)" \
 	  --owner "$(OWNER)" \
-	  --variant "$(VARIANT)"
+	  --short-image-name "$(notdir $@)" \
+	  --variant "$(VARIANT)" \
+	  --tags-dir /tmp/jupyter/tags/
 	python3 -m tagging.write_manifest \
+	  --registry "$(REGISTRY)" \
+	  --owner "$(OWNER)" \
 	  --short-image-name "$(notdir $@)" \
+	  --variant "$(VARIANT)" \
 	  --hist-lines-dir /tmp/jupyter/hist_lines/ \
-	  --manifests-dir /tmp/jupyter/manifests/ \
+	  --manifests-dir /tmp/jupyter/manifests/
-	  --registry "$(REGISTRY)" \
-	  --owner "$(OWNER)" \
-	  --variant "$(VARIANT)"
 	python3 -m tagging.apply_tags \
-	  --short-image-name "$(notdir $@)" \
-	  --tags-dir /tmp/jupyter/tags/ \
-	  --platform "$(shell uname -m)" \
 	  --registry "$(REGISTRY)" \
 	  --owner "$(OWNER)" \
-	  --variant "$(VARIANT)"
+	  --short-image-name "$(notdir $@)" \
+	  --variant "$(VARIANT)" \
+	  --platform "$(shell uname -m)" \
+	  --tags-dir /tmp/jupyter/tags/
 hook-all: $(foreach I, $(ALL_IMAGES), hook/$(I)) ## run post-build hooks for all images
 
 
@@ -135,7 +135,7 @@ run-sudo-shell/%: ## run bash in interactive mode as root in a stack
 
 test/%: ## run tests against a stack
 	python3 -m tests.run_tests \
-	  --short-image-name "$(notdir $@)" \
 	  --registry "$(REGISTRY)" \
-	  --owner "$(OWNER)"
+	  --owner "$(OWNER)" \
+	  --short-image-name "$(notdir $@)"
 test-all: $(foreach I, $(ALL_IMAGES), test/$(I)) ## test all stacks

tagging/apply_tags.py

@@ -1,12 +1,12 @@
 #!/usr/bin/env python3
 # Copyright (c) Jupyter Development Team.
 # Distributed under the terms of the Modified BSD License.
-import argparse
 import logging
 from pathlib import Path
 
 import plumbum
 
+from tagging.common_arguments import common_arguments_parser
 from tagging.get_platform import unify_aarch64
 from tagging.get_prefix import get_file_prefix_for_platform
 
@@ -16,12 +16,13 @@ LOGGER = logging.getLogger(__name__)
 
 
 def apply_tags(
-    short_image_name: str,
+    *,
     registry: str,
     owner: str,
-    tags_dir: Path,
+    short_image_name: str,
-    platform: str,
     variant: str,
+    platform: str,
+    tags_dir: Path,
 ) -> None:
     """
     Tags <registry>/<owner>/<short_image_name>:latest with the tags reported by all taggers for this image
@@ -41,18 +42,7 @@ def apply_tags(
 if __name__ == "__main__":
     logging.basicConfig(level=logging.INFO)
 
-    arg_parser = argparse.ArgumentParser()
+    arg_parser = common_arguments_parser()
-    arg_parser.add_argument(
-        "--short-image-name",
-        required=True,
-        help="Short image name",
-    )
-    arg_parser.add_argument(
-        "--tags-dir",
-        required=True,
-        type=Path,
-        help="Directory with saved tags file",
-    )
     arg_parser.add_argument(
         "--platform",
         required=True,
@@ -61,30 +51,12 @@ if __name__ == "__main__":
         help="Image platform",
     )
     arg_parser.add_argument(
-        "--registry",
+        "--tags-dir",
         required=True,
-        type=str,
+        type=Path,
-        choices=["docker.io", "quay.io"],
+        help="Directory with saved tags file",
-        help="Image registry",
-    )
-    arg_parser.add_argument(
-        "--owner",
-        required=True,
-        help="Owner of the image",
-    )
-    arg_parser.add_argument(
-        "--variant",
-        required=True,
-        help="Variant tag prefix",
     )
     args = arg_parser.parse_args()
     args.platform = unify_aarch64(args.platform)
 
-    apply_tags(
+    apply_tags(**vars(args))
-        args.short_image_name,
-        args.registry,
-        args.owner,
-        args.tags_dir,
-        args.platform,
-        args.variant,
-    )

tagging/common_arguments.py

@@ -0,0 +1,39 @@
+import argparse
+
+
+def common_arguments_parser(
+    registry: bool = True,
+    owner: bool = True,
+    short_image_name: bool = True,
+    variant: bool = True,
+) -> argparse.ArgumentParser:
+    """Add common CLI arguments to parser"""
+
+    parser = argparse.ArgumentParser()
+    if registry:
+        parser.add_argument(
+            "--registry",
+            required=True,
+            choices=["docker.io", "quay.io"],
+            help="Image registry",
+        )
+    if owner:
+        parser.add_argument(
+            "--owner",
+            required=True,
+            help="Owner of the image",
+        )
+    if short_image_name:
+        parser.add_argument(
+            "--short-image-name",
+            required=True,
+            help="Short image name",
+        )
+    if variant:
+        parser.add_argument(
+            "--variant",
+            required=True,
+            help="Variant tag prefix",
+        )
+
+    return parser

tagging/merge_tags.py

@@ -1,12 +1,12 @@
 #!/usr/bin/env python3
 # Copyright (c) Jupyter Development Team.
 # Distributed under the terms of the Modified BSD License.
-import argparse
 import logging
 from pathlib import Path
 
 import plumbum
 
+from tagging.common_arguments import common_arguments_parser
 from tagging.get_platform import ALL_PLATFORMS
 from tagging.get_prefix import get_file_prefix_for_platform
 
@@ -16,6 +16,7 @@ LOGGER = logging.getLogger(__name__)
 
 
 def merge_tags(
+    *,
     short_image_name: str,
     variant: str,
     tags_dir: Path,
@@ -60,23 +61,13 @@ def merge_tags(
 if __name__ == "__main__":
     logging.basicConfig(level=logging.INFO)
 
-    arg_parser = argparse.ArgumentParser()
+    arg_parser = common_arguments_parser(registry=False, owner=False)
-    arg_parser.add_argument(
-        "--short-image-name",
-        required=True,
-        help="Short image name",
-    )
     arg_parser.add_argument(
         "--tags-dir",
         required=True,
         type=Path,
         help="Directory with saved tags file",
     )
-    arg_parser.add_argument(
-        "--variant",
-        required=True,
-        help="Variant tag prefix",
-    )
     args = arg_parser.parse_args()
 
-    merge_tags(args.short_image_name, args.variant, args.tags_dir)
+    merge_tags(**vars(args))

tagging/update_wiki.py

@@ -155,7 +155,11 @@ def remove_old_manifests(wiki_dir: Path) -> None:
 
 
 def update_wiki(
-    wiki_dir: Path, hist_lines_dir: Path, manifests_dir: Path, allow_no_files: bool
+    *,
+    wiki_dir: Path,
+    hist_lines_dir: Path,
+    manifests_dir: Path,
+    allow_no_files: bool,
 ) -> None:
     LOGGER.info("Updating wiki")
 
@@ -214,6 +218,4 @@ if __name__ == "__main__":
     )
     args = arg_parser.parse_args()
 
-    update_wiki(
+    update_wiki(**vars(args))
-        args.wiki_dir, args.hist_lines_dir, args.manifests_dir, args.allow_no_files
-    )

tagging/write_manifest.py

@@ -1,13 +1,13 @@
 #!/usr/bin/env python3
 # Copyright (c) Jupyter Development Team.
 # Distributed under the terms of the Modified BSD License.
-import argparse
 import datetime
 import logging
 from pathlib import Path
 
 from docker.models.containers import Container
 
+from tagging.common_arguments import common_arguments_parser
 from tagging.docker_runner import DockerRunner
 from tagging.get_prefix import get_file_prefix, get_tag_prefix
 from tagging.get_taggers_and_manifests import get_taggers_and_manifests
@@ -22,9 +22,10 @@ MARKDOWN_LINE_BREAK = "<br />"
 
 
 def write_build_history_line(
-    short_image_name: str,
+    *,
     registry: str,
     owner: str,
+    short_image_name: str,
     hist_lines_dir: Path,
     filename: str,
     all_tags: list[str],
@@ -49,9 +50,10 @@ def write_build_history_line(
 
 
 def write_manifest_file(
-    short_image_name: str,
+    *,
     registry: str,
     owner: str,
+    short_image_name: str,
     manifests_dir: Path,
     filename: str,
     manifests: list[ManifestInterface],
@@ -70,9 +72,10 @@ def write_manifest_file(
 
 
 def write_manifest(
-    short_image_name: str,
+    *,
     registry: str,
     owner: str,
+    short_image_name: str,
     variant: str,
     hist_lines_dir: Path,
     manifests_dir: Path,
@@ -92,28 +95,28 @@ def write_manifest(
             tags_prefix + "-" + tagger.tag_value(container) for tagger in taggers
         ]
         write_build_history_line(
-            short_image_name, registry, owner, hist_lines_dir, filename, all_tags
+            registry=registry,
+            owner=owner,
+            short_image_name=short_image_name,
+            hist_lines_dir=hist_lines_dir,
+            filename=filename,
+            all_tags=all_tags,
         )
         write_manifest_file(
-            short_image_name,
+            registry=registry,
-            registry,
+            owner=owner,
-            owner,
+            short_image_name=short_image_name,
-            manifests_dir,
+            manifests_dir=manifests_dir,
-            filename,
+            filename=filename,
-            manifests,
+            manifests=manifests,
-            container,
+            container=container,
         )
 
 
 if __name__ == "__main__":
     logging.basicConfig(level=logging.INFO)
 
-    arg_parser = argparse.ArgumentParser()
+    arg_parser = common_arguments_parser()
-    arg_parser.add_argument(
-        "--short-image-name",
-        required=True,
-        help="Short image name",
-    )
     arg_parser.add_argument(
         "--hist-lines-dir",
         required=True,
@@ -126,32 +129,8 @@ if __name__ == "__main__":
         type=Path,
         help="Directory to save manifest file",
     )
-    arg_parser.add_argument(
-        "--registry",
-        required=True,
-        type=str,
-        choices=["docker.io", "quay.io"],
-        help="Image registry",
-    )
-    arg_parser.add_argument(
-        "--owner",
-        required=True,
-        help="Owner of the image",
-    )
-    arg_parser.add_argument(
-        "--variant",
-        required=True,
-        help="Variant tag prefix",
-    )
     args = arg_parser.parse_args()
 
     LOGGER.info(f"Current build timestamp: {BUILD_TIMESTAMP}")
 
-    write_manifest(
+    write_manifest(**vars(args))
-        args.short_image_name,
-        args.registry,
-        args.owner,
-        args.variant,
-        args.hist_lines_dir,
-        args.manifests_dir,
-    )

tagging/write_tags_file.py

@@ -1,10 +1,10 @@
 #!/usr/bin/env python3
 # Copyright (c) Jupyter Development Team.
 # Distributed under the terms of the Modified BSD License.
-import argparse
 import logging
 from pathlib import Path
 
+from tagging.common_arguments import common_arguments_parser
 from tagging.docker_runner import DockerRunner
 from tagging.get_prefix import get_file_prefix, get_tag_prefix
 from tagging.get_taggers_and_manifests import get_taggers_and_manifests
@@ -13,9 +13,10 @@ LOGGER = logging.getLogger(__name__)
 
 
 def write_tags_file(
-    short_image_name: str,
+    *,
     registry: str,
     owner: str,
+    short_image_name: str,
     variant: str,
     tags_dir: Path,
 ) -> None:
@@ -48,41 +49,13 @@ def write_tags_file(
 if __name__ == "__main__":
     logging.basicConfig(level=logging.INFO)
 
-    arg_parser = argparse.ArgumentParser()
+    arg_parser = common_arguments_parser()
-    arg_parser.add_argument(
-        "--short-image-name",
-        required=True,
-        help="Short image name",
-    )
     arg_parser.add_argument(
         "--tags-dir",
         required=True,
         type=Path,
         help="Directory to save tags file",
     )
-    arg_parser.add_argument(
-        "--registry",
-        required=True,
-        type=str,
-        choices=["docker.io", "quay.io"],
-        help="Image registry",
-    )
-    arg_parser.add_argument(
-        "--owner",
-        required=True,
-        help="Owner of the image",
-    )
-    arg_parser.add_argument(
-        "--variant",
-        required=True,
-        help="Variant tag prefix",
-    )
     args = arg_parser.parse_args()
 
-    write_tags_file(
+    write_tags_file(**vars(args))
-        args.short_image_name,
-        args.registry,
-        args.owner,
-        args.variant,
-        args.tags_dir,
-    )

tests/run_tests.py

@@ -13,7 +13,12 @@ python3 = plumbum.local["python3"]
 LOGGER = logging.getLogger(__name__)
 
 
-def test_image(short_image_name: str, registry: str, owner: str) -> None:
+def test_image(
+    *,
+    registry: str,
+    owner: str,
+    short_image_name: str,
+) -> None:
     LOGGER.info(f"Testing image: {short_image_name}")
     test_dirs = get_test_dirs(short_image_name)
     LOGGER.info(f"Test dirs to be run: {test_dirs}")
@@ -36,15 +41,9 @@ if __name__ == "__main__":
     logging.basicConfig(level=logging.INFO)
 
     arg_parser = argparse.ArgumentParser()
-    arg_parser.add_argument(
-        "--short-image-name",
-        required=True,
-        help="Short image name",
-    )
     arg_parser.add_argument(
         "--registry",
         required=True,
-        type=str,
         choices=["docker.io", "quay.io"],
         help="Image registry",
     )
@@ -53,7 +52,11 @@ if __name__ == "__main__":
         required=True,
         help="Owner of the image",
     )
-
+    arg_parser.add_argument(
+        "--short-image-name",
+        required=True,
+        help="Short image name",
+    )
     args = arg_parser.parse_args()
 
-    test_image(args.short_image_name, args.registry, args.owner)
+    test_image(**vars(args))