Small rework of the pypi cleanup script

Author: evertlammerts

.github/workflows/cleanup_pypi.yml

@@ -6,6 +6,14 @@ on:
         description: CI environment to run in (pypi-test or pypi-prod-nightly)
         type: string
         required: true
+      subcommand:
+        description: List or Delete
+        type: string
+        default: delete
+      verbosity:
+        description: Tool verbosity ("verbose" or "debug")
+        type: string
+        default: verbose
     secrets:
       PYPI_CLEANUP_OTP:
         description: PyPI OTP
@@ -15,17 +23,29 @@ on:
         required: true
   workflow_dispatch:
     inputs:
-      dry-run:
-        description: List packages that would be deleted but don't delete them
-        type: boolean
-        default: false
+      subcommand:
+        description: List or Delete
+        type: choice
+        required: true
+        options:
+          - list
+          - delete
+        default: list
       environment:
         description: CI environment to run in
         type: choice
         required: true
         options:
           - pypi-prod-nightly
           - pypi-test
+      verbosity:
+        description: Tool verbosity
+        type: choice
+        required: true
+        options:
+          - verbose
+          - debug
+        default: verbose
 
 jobs:
   cleanup_pypi:
@@ -54,22 +74,37 @@ jobs:
         with:
           version: "0.9.0"
 
-      - name: Run Cleanup
+      - name: Install dependencies
+        run: uv sync --only-group pypi --no-install-project
+
+      - name: List Stale Packages on PyPI
+        if: inputs.subcommand == 'list'
+        env:
+          PYTHON_UNBUFFERED: 1
+        run: |
+          set -x
+          uv run --no-sync python -u -m duckdb_packaging.pypi_cleanup \
+            --${{ inputs.environment == 'pypi-prod-nightly' && 'prod' || 'test' }} \
+            --max-nightlies ${{ vars.PYPI_MAX_NIGHTLIES }} \
+            --${{ inputs.verbosity }} \
+            list  2>&1 | tee cleanup_output
+
+      - name: Delete Stale Packages from PyPI
+        if: inputs.subcommand == 'delete'
         env:
           PYTHON_UNBUFFERED: 1
         run: |
           set -x
-          uv sync --only-group pypi --no-install-project
-          uv run --no-sync python -u -m duckdb_packaging.pypi_cleanup ${{ inputs.dry-run && '--dry' || '' }} \
-            ${{ inputs.environment == 'pypi-prod-nightly' && '--prod' || '--test' }} \
-            --verbose \
-            --username "${{ vars.PYPI_CLEANUP_USERNAME }}" \
-            --max-nightlies ${{ vars.PYPI_MAX_NIGHTLIES }} 2>&1 | tee cleanup_output
+          uv run --no-sync python -u -m duckdb_packaging.pypi_cleanup \
+            --${{ inputs.environment == 'pypi-prod-nightly' && 'prod' || 'test' }} \
+            --max-nightlies ${{ vars.PYPI_MAX_NIGHTLIES }} \
+            --${{ inputs.verbosity }} \
+            delete --username "${{ vars.PYPI_CLEANUP_USERNAME }}" 2>&1 | tee cleanup_output
 
       - name: PyPI Cleanup Summary
         run : |
           echo "## PyPI Cleanup Summary" >> $GITHUB_STEP_SUMMARY
-          echo "* Dry run: ${{ inputs.dry-run }}" >> $GITHUB_STEP_SUMMARY
+          echo "* Subcommand: ${{ inputs.subcommand }}" >> $GITHUB_STEP_SUMMARY
           echo "* CI Environment: ${{ inputs.environment }}" >> $GITHUB_STEP_SUMMARY
           echo "* Output:" >> $GITHUB_STEP_SUMMARY
           echo '```' >> $GITHUB_STEP_SUMMARY

duckdb_packaging/pypi_cleanup.py

@@ -38,6 +38,14 @@
 
 def create_argument_parser() -> argparse.ArgumentParser:
     """Create and configure the argument parser."""
+
+    def max_nightlies_type(value: int) -> int:
+        """Validate that --max-nightlies is set to a positive integer."""
+        if int(value) < 0:
+            msg = f"max-nightlies must be a positive integer, got {int(value)}"
+            raise ValueError(msg)
+        return int(value)
+
     parser = argparse.ArgumentParser(
         description="""
 PyPI cleanup script for removing development versions.
@@ -54,23 +62,54 @@ def create_argument_parser() -> argparse.ArgumentParser:
         formatter_class=argparse.RawDescriptionHelpFormatter,
     )
 
-    parser.add_argument("--dry-run", action="store_true", help="Show what would be deleted but don't actually do it")
+    loglevel_group = parser.add_mutually_exclusive_group(required=False)
+    loglevel_group.add_argument(
+        "-d",
+        "--debug",
+        help="Show debug logs",
+        dest="loglevel",
+        action="store_const",
+        const=logging.DEBUG,
+        default=logging.WARNING,
+    )
+    loglevel_group.add_argument(
+        "-v",
+        "--verbose",
+        help="Show info logs",
+        dest="loglevel",
+        action="store_const",
+        const=logging.INFO,
+    )
 
     host_group = parser.add_mutually_exclusive_group(required=True)
-    host_group.add_argument("--prod", action="store_true", help="Use production PyPI (pypi.org)")
-    host_group.add_argument("--test", action="store_true", help="Use test PyPI (test.pypi.org)")
+    host_group.add_argument(
+        "--prod", help="Use production PyPI (pypi.org)", dest="pypi_url", action="store_const", const=_PYPI_URL_PROD
+    )
+    host_group.add_argument(
+        "--test", help="Use test PyPI (test.pypi.org)", dest="pypi_url", action="store_const", const=_PYPI_URL_TEST
+    )
 
     parser.add_argument(
         "-m",
         "--max-nightlies",
-        type=int,
+        type=max_nightlies_type,
         default=_DEFAULT_MAX_NIGHTLIES,
         help=f"Max number of nightlies of unreleased versions (default={_DEFAULT_MAX_NIGHTLIES})",
     )
 
-    parser.add_argument("-u", "--username", type=validate_username, help="PyPI username (required unless --dry-run)")
+    subparsers = parser.add_subparsers(title="Subcommands")
 
-    parser.add_argument("-v", "--verbose", action="store_true", help="Enable verbose debug logging")
+    # Add the "list" subcommand
+    parser_list = subparsers.add_parser("list", help="List all packages available for deletion")
+    parser_list.set_defaults(func=lambda args: _run(CleanMode.LIST_ONLY, args))
+    # Add the "delete" subcommand
+    parser_delete = subparsers.add_parser(
+        "delete", help="Delete packages that match the given criteria (use with care!"
+    )
+    parser_delete.add_argument(
+        "-u", "--username", type=validate_username, help="PyPI username (required)", required=True
+    )
+    parser_delete.set_defaults(func=lambda args: _run(CleanMode.DELETE, args))
 
     return parser
 
@@ -149,17 +188,6 @@ def load_credentials() -> tuple[Optional[str], Optional[str]]:
     return password, otp
 
 
-def validate_arguments(args: argparse.Namespace) -> None:
-    """Validate parsed arguments."""
-    if not args.dry_run and not args.username:
-        msg = "--username is required when not in dry-run mode"
-        raise ValidationError(msg)
-
-    if args.max_nightlies < 0:
-        msg = "--max-nightlies must be non-negative"
-        raise ValidationError(msg)
-
-
 class CsrfParser(HTMLParser):
     """HTML parser to extract CSRF tokens from PyPI forms.
 
@@ -230,7 +258,7 @@ def run(self) -> int:
         if self._mode == CleanMode.DELETE:
             logging.warning("NOT A DRILL: WILL DELETE PACKAGES")
         elif self._mode == CleanMode.LIST_ONLY:
-            logging.info("Running in DRY RUN mode, nothing will be deleted")
+            logging.debug("Running in DRY RUN mode, nothing will be deleted")
         else:
             msg = "Unexpected mode"
             raise RuntimeError(msg)
@@ -255,20 +283,22 @@ def _execute_cleanup(self, http_session: Session) -> int:
             logging.info(f"No releases found for {self._package}")
             return 0
 
-        # Determine versions to delete
+        # Determine and report versions to delete
         versions_to_delete = self._determine_versions_to_delete(versions)
-        if not versions_to_delete:
-            logging.info("No versions to delete (no stale rc's or dev releases)")
+        if len(versions_to_delete) > 0:
+            print(f"Found the following stale releases on {self._index_host}:")
+            for version in sorted(versions_to_delete):
+                print(f"- {version}")
+        else:
+            print(f"No stale releases found on {self._index_host}")
             return 0
 
-        logging.warning(f"Found {len(versions_to_delete)} versions to clean up:")
-        for version in sorted(versions_to_delete):
-            logging.warning(version)
-
         if self._mode != CleanMode.DELETE:
             logging.info("Dry run complete - no packages were deleted")
             return 0
 
+        logging.warning(f"Will try to delete {len(versions_to_delete)} releases from {self._index_host}")
+
         # Perform authentication and deletion
         self._authenticate(http_session)
         self._delete_versions(http_session, versions_to_delete)
@@ -527,42 +557,39 @@ def _delete_single_version(self, http_session: Session, version: str) -> None:
         delete_response.raise_for_status()
 
 
-def main() -> int:
-    """Main entry point for the script."""
-    parser = create_argument_parser()
-    args = parser.parse_args()
-
-    # Setup logging
-    setup_logging((args.verbose and logging.DEBUG) or logging.INFO)
-
+def _run(mode: CleanMode, args: argparse.Namespace) -> int:
+    """Action called by the subcommands after arg parsing."""
+    setup_logging(args.loglevel)
     try:
-        # Validate arguments
-        validate_arguments(args)
-
-        # Dry run vs delete
-        password, otp, mode = None, None, CleanMode.LIST_ONLY
-        if not args.dry_run:
+        if mode == CleanMode.DELETE:
             password, otp = load_credentials()
-            mode = CleanMode.DELETE
-
-        # Determine PyPI URL
-        pypi_url = _PYPI_URL_PROD if args.prod else _PYPI_URL_TEST
-
-        # Create and run cleanup
-        cleanup = PyPICleanup(pypi_url, mode, args.max_nightlies, username=args.username, password=password, otp=otp)
-
+            cleanup = PyPICleanup(
+                args.pypi_url, mode, args.max_nightlies, username=args.username, password=password, otp=otp
+            )
+        elif mode == CleanMode.LIST_ONLY:
+            cleanup = PyPICleanup(args.pypi_url, mode, args.max_nightlies)
+        else:
+            print(f"Unknown mode {mode}. Did nothing.")
+            return -1
         return cleanup.run()
-
     except ValidationError:
         logging.exception("Configuration error")
         return 2
     except KeyboardInterrupt:
         logging.info("Operation cancelled by user")
         return 130
     except Exception:
-        logging.exception("Unexpected error", exc_info=args.verbose)
+        logging.exception("Unexpected error")
         return 1
 
 
+def main() -> int:
+    """Main entry point for the script."""
+    parser = create_argument_parser()
+    args = parser.parse_args()
+    # call the subcommand's func
+    return args.func(args)
+
+
 if __name__ == "__main__":
     sys.exit(main())

tests/fast/test_pypi_cleanup.py

@@ -15,6 +15,8 @@
 duckdb_packaging = pytest.importorskip("duckdb_packaging")
 
 from duckdb_packaging.pypi_cleanup import (  # noqa: E402
+    _DEFAULT_MAX_NIGHTLIES,
+    _PYPI_URL_PROD,
     AuthenticationError,
     CleanMode,
     CsrfParser,
@@ -26,7 +28,6 @@
     main,
     session_with_retries,
     setup_logging,
-    validate_arguments,
     validate_username,
 )
 
@@ -60,23 +61,6 @@ def test_validate_username_invalid(self):
         with pytest.raises(ArgumentTypeError, match="Invalid username format"):
             validate_username("invalid-")
 
-    def test_validate_arguments_dry_run(self):
-        """Test argument validation for dry run mode."""
-        args = Mock(dry_run=True, username=None, max_nightlies=2)
-        validate_arguments(args)  # Should not raise
-
-    def test_validate_arguments_live_mode_no_username(self):
-        """Test argument validation for live mode without username."""
-        args = Mock(dry_run=False, username=None, max_nightlies=2)
-        with pytest.raises(ValidationError, match="username is required"):
-            validate_arguments(args)
-
-    def test_validate_arguments_negative_nightlies(self):
-        """Test argument validation with negative max nightlies."""
-        args = Mock(dry_run=True, username="test", max_nightlies=-1)
-        with pytest.raises(ValidationError, match="must be non-negative"):
-            validate_arguments(args)
-
 
 class TestCredentials:
     """Test credential loading."""
@@ -465,26 +449,20 @@ def test_argument_parser_creation(self):
         parser = create_argument_parser()
         assert parser.prog is not None
 
-    def test_parse_args_prod_dry_run(self):
+    def test_parse_args_prod_list(self):
         """Test parsing arguments for production dry run."""
         parser = create_argument_parser()
-        args = parser.parse_args(["--prod", "--dry-run"])
+        args = parser.parse_args(["--prod", "list"])
 
-        assert args.prod is True
-        assert args.test is False
-        assert args.dry_run is True
-        assert args.max_nightlies == 2
-        assert args.verbose is False
+        assert args.pypi_url is _PYPI_URL_PROD
+        assert args.max_nightlies == _DEFAULT_MAX_NIGHTLIES
+        assert args.loglevel is logging.WARN
 
-    def test_parse_args_test_with_username(self):
+    def test_parse_args_test_list_with_username(self):
         """Test parsing arguments for test with username."""
         parser = create_argument_parser()
-        args = parser.parse_args(["--test", "-u", "testuser", "--verbose"])
-
-        assert args.test is True
-        assert args.prod is False
-        assert args.username == "testuser"
-        assert args.verbose is True
+        with pytest.raises(SystemExit):
+            parser.parse_args(["--test", "list", "-u", "testuser"])
 
     def test_parse_args_missing_host(self):
         """Test parsing arguments with missing host selection."""
@@ -506,28 +484,9 @@ def test_main_success(self, mock_cleanup_class, mock_setup_logging):
         mock_cleanup.run.return_value = 0
         mock_cleanup_class.return_value = mock_cleanup
 
-        with patch("sys.argv", ["pypi_cleanup.py", "--test", "-u", "testuser"]):
+        with patch("sys.argv", ["pypi_cleanup.py", "--test", "delete", "-u", "testuser"]):
             result = main()
 
         assert result == 0
         mock_setup_logging.assert_called_once()
         mock_cleanup.run.assert_called_once()
-
-    @patch("duckdb_packaging.pypi_cleanup.setup_logging")
-    def test_main_validation_error(self, mock_setup_logging):
-        """Test main function with validation error."""
-        with patch("sys.argv", ["pypi_cleanup.py", "--test"]):  # Missing username for live mode
-            result = main()
-
-        assert result == 2  # Validation error exit code
-
-    @patch("duckdb_packaging.pypi_cleanup.setup_logging")
-    @patch("duckdb_packaging.pypi_cleanup.validate_arguments")
-    def test_main_keyboard_interrupt(self, mock_validate, mock_setup_logging):
-        """Test main function with keyboard interrupt."""
-        mock_validate.side_effect = KeyboardInterrupt()
-
-        with patch("sys.argv", ["pypi_cleanup.py", "--test", "--dry-run"]):
-            result = main()
-
-        assert result == 130  # Keyboard interrupt exit code