Merge pull request #129 from lsst-sqre/tickets/DM-42231

Further typing tightening/modernization

.pre-commit-config.yaml

@@ -28,5 +28,5 @@ repos:
     rev: 1.16.0
     hooks:
       - id: blacken-docs
-        additional_dependencies: [black==23.10.1]
-        args: [-l, '79', -t, py311]
+        additional_dependencies: [black==23.12.1]
+        args: [-l, '79', -t, py310]

.readthedocs.yaml

@@ -15,6 +15,6 @@ formats:
 
 # Optionally set the version of Python and requirements required to build your docs
 python:
-   version: 3.7
+   version: 3.10
    install:
-   - requirements: dev-requirements.txt
+   - requirements: requirements/dev.txt

.travis.yml

@@ -9,7 +9,7 @@ env:
     - CC_TEST_REPORTER_ID=cca5a6743728de037cb47d4a845e35c682b4469c0f9c52851f4f3824dd471f87
 
 install:
-  - pip install -r requirements.txt
+  - pip install -r requirements/main.txt
 
 before_script:
   - curl -L https://codeclimate.com/downloads/test-reporter/test-reporter-latest-linux-amd64 > ./cc-test-reporter

Dockerfile

@@ -14,15 +14,15 @@ RUN mkdir /wheels
 ARG UWSGI_VERSION=2.0.23
 RUN pip wheel -w /wheels uwsgi==$UWSGI_VERSION
 
-COPY requirements.txt /
+COPY requirements/main.txt /requirements.txt
 RUN pip wheel -w /wheels -r /requirements.txt
 
 ### --- Build Final Image ---
 
 FROM python:3.10-slim
 
 RUN DEBIAN_FRONTEND=noninteractive apt-get update \
-    && apt-get install -y libpcre3 libxml2 tini \
+    && apt-get install -y libpcre3 libxml2 tini git \
     && apt-get clean \
     && apt -y autoremove
 

MANIFEST.in

@@ -1,2 +1 @@
-include VERSION
 include README.md

Makefile

@@ -2,7 +2,6 @@
 PACKAGE_NAME := giftless
 PACKAGE_DIRS := giftless
 TESTS_DIR := tests
-VERSION_FILE := VERSION
 
 SHELL := bash
 PYTHON := python
@@ -12,24 +11,29 @@ PYTEST := pytest
 DOCKER := docker
 GIT := git
 
+DOCKER_HOST := docker.io
 DOCKER_REPO := datopian
 DOCKER_IMAGE_NAME := giftless
 DOCKER_IMAGE_TAG := latest
-DOCKER_CACHE_FROM := datopian/giftless:latest
+DOCKER_CACHE_FROM := docker.io/datopian/giftless:latest
 
 PYTEST_EXTRA_ARGS :=
 
-VERSION := $(shell cat $(VERSION_FILE))
 SOURCE_FILES := $(shell find $(PACKAGE_DIRS) $(TESTS_DIR) -type f -name "*.py")
 SENTINELS := .make-cache
 DIST_DIR := dist
 
 PYVER := $(shell $(PYTHON) -c "import sys;print(f'{sys.version_info[0]}{sys.version_info[1]}')")
+VERSION := $(shell $(PYTHON) -c "from importlib.metadata import version;print(version('$(PACKAGE_NAME)'))")
 
 default: help
 
+## Install packages necessary for make to work
+init:
+	pip install --upgrade pip pre-commit pip-tools tox
+
 ## Regenerate requirements files
-requirements: dev-requirements.txt dev-requirements.in
+requirements: requirements/dev.txt requirements/dev.in requirements/main.txt requirements/main.in
 
 ## Set up the development environment
 dev-setup: $(SENTINELS)/dev-setup
@@ -39,22 +43,12 @@ test: $(SENTINELS)/dev-setup
 	$(PYTEST) $(PYTEST_EXTRA_ARGS) $(PACKAGE_DIRS) $(TESTS_DIR)
 
 ## Build a local Docker image
-docker: requirements.txt
-	$(DOCKER) build --cache-from "$(DOCKER_CACHE_FROM)" -t $(DOCKER_REPO)/$(DOCKER_IMAGE_NAME):$(DOCKER_IMAGE_TAG) .
-
-## Tag and push a release
-release: $(SENTINELS)/dist
-	@echo
-	@echo "You are about to release $(PACKAGE_NAME) version $(VERSION)"
-	@echo "This will:"
-	@echo " - Create and push a git tag v$(VERSION)"
-	@echo " - Create a release package and upload it to pypi"
-	@echo
-	@echo "Continue? (hit Enter or Ctrl+C to stop"
-	@read
-	$(GIT) tag v$(VERSION)
-	$(GIT) push --tags
-	$(PYTHON) -m twine upload dist/*
+docker: requirements/main.txt
+	$(DOCKER) build --cache-from "$(DOCKER_CACHE_FROM)" -t $(DOCKER_HOST)/$(DOCKER_REPO)/$(DOCKER_IMAGE_NAME):$(DOCKER_IMAGE_TAG) .
+
+## Tag and push a release (disabled)
+release:
+	@echo "Package '$(PACKAGE_NAME)' releases are managed via GitHub"
 
 ## Clean all generated files
 distclean:
@@ -68,16 +62,16 @@ dist: $(SENTINELS)/dist
 docs-html:
 	@cd docs && $(MAKE) html
 
-.PHONY: test docker release dist distclean requirements docs-html
+.PHONY: test docker release dist distclean requirements docs-html init
 
-requirements.txt: requirements.in
-	$(PIP_COMPILE) --no-emit-index-url -o requirements.txt requirements.in
+requirements/main.txt: requirements/main.in
+	$(PIP_COMPILE) --no-emit-index-url -o requirements/main.txt requirements/main.in
 
-dev-requirements.txt: dev-requirements.in requirements.txt
-	$(PIP_COMPILE) --no-emit-index-url -o dev-requirements.txt dev-requirements.in
+requirements/dev.txt: requirements/dev.in requirements/main.txt
+	$(PIP_COMPILE) --no-emit-index-url -o requirements/dev.txt requirements/dev.in
 
-$(DIST_DIR)/$(PACKAGE_NAME)-$(VERSION).tar.gz $(DIST_DIR)/$(PACKAGE_NAME)-$(VERSION)-py3-none-any.whl: $(SOURCE_FILES) setup.py VERSION README.md | $(SENTINELS)/dist-setup
-	$(PYTHON) setup.py sdist bdist_wheel
+$(DIST_DIR)/$(PACKAGE_NAME)-$(VERSION).tar.gz $(DIST_DIR)/$(PACKAGE_NAME)-$(VERSION)-py3-none-any.whl: $(SOURCE_FILES) README.md | $(SENTINELS)/dist-setup
+	$(PYTHON) -m build
 
 $(SENTINELS):
 	mkdir $@
@@ -89,10 +83,10 @@ $(SENTINELS)/dist-setup: | $(SENTINELS)
 $(SENTINELS)/dist: $(SENTINELS)/dist-setup $(DIST_DIR)/$(PACKAGE_NAME)-$(VERSION).tar.gz $(DIST_DIR)/$(PACKAGE_NAME)-$(VERSION)-py3-none-any.whl | $(SENTINELS)
 	@touch $@
 
-$(SENTINELS)/dev-setup: requirements.txt dev-requirements.txt | $(SENTINELS)
-	$(PIP) install -r requirements.txt
+$(SENTINELS)/dev-setup: requirements/main.txt requirements/dev.txt | $(SENTINELS)
+	$(PIP) install -r requirements/main.txt
 	$(PIP) install -e .
-	$(PIP) install -r dev-requirements.txt
+	$(PIP) install -r requirements/dev.txt
 	@touch $@
 
 # Help related variables and targets

docs/source/conf.py

@@ -11,6 +11,7 @@
 # documentation root, use os.path.abspath to make it absolute, like shown here.
 #
 import os
+import importlib
 
 from recommonmark.transform import AutoStructify
 
@@ -25,9 +26,7 @@
 author = "Shahar Evron"
 
 # The full version, including alpha/beta/rc tags
-with open(os.path.join(os.path.dirname(__file__), "..", "..", "VERSION")) as f:
-    release = f.read().strip()
-
+release = importlib.metadata.version(project)
 
 # -- General configuration ---------------------------------------------------
 

giftless/app.py

@@ -2,17 +2,18 @@
 """
 import logging
 import os
+from typing import Any
 
 from flask import Flask
-from flask_marshmallow import Marshmallow  # type: ignore
+from flask_marshmallow import Marshmallow
 
 from giftless import config, transfer, view
 from giftless.auth import authentication
 from giftless.error_handling import ApiErrorHandler
 from giftless.util import get_callable
 
 
-def init_app(app=None, additional_config=None):
+def init_app(app: Flask | None = None, additional_config: Any = None) -> Flask:
     """Flask app initialization"""
     if app is None:
         app = Flask(__name__)

giftless/auth/__init__.py

@@ -4,9 +4,9 @@
 import logging
 from collections.abc import Callable
 from functools import wraps
-from typing import Any, Optional, Union
+from typing import Any, Union
 
-from flask import Request, current_app, g
+from flask import Flask, Request, current_app, g
 from flask import request as flask_request
 from typing_extensions import Protocol
 from werkzeug.exceptions import Unauthorized as BaseUnauthorized
@@ -27,7 +27,7 @@ class Authenticator(Protocol):
     a request and provide an identity object
     """
 
-    def __call__(self, request: Request) -> Optional[Identity]:
+    def __call__(self, request: Request) -> Identity | None:
         raise NotImplementedError(
             "This is a protocol definition and should not be called directly"
         )
@@ -46,9 +46,9 @@ def get_authz_query_params(
         identity: Identity,
         org: str,
         repo: str,
-        actions: Optional[set[str]] = None,
-        oid: Optional[str] = None,
-        lifetime: Optional[int] = None,
+        actions: set[str] | None = None,
+        oid: str | None = None,
+        lifetime: int | None = None,
     ) -> dict[str, str]:
         """Authorize an action by adding credientaisl to the query string"""
         return {}
@@ -58,32 +58,34 @@ def get_authz_header(
         identity: Identity,
         org: str,
         repo: str,
-        actions: Optional[set[str]] = None,
-        oid: Optional[str] = None,
-        lifetime: Optional[int] = None,
+        actions: set[str] | None = None,
+        oid: str | None = None,
+        lifetime: int | None = None,
     ) -> dict[str, str]:
         """Authorize an action by adding credentials to the request headers"""
         return {}
 
 
 class Authentication:
     def __init__(
-        self, app=None, default_identity: Optional[Identity] = None
+        self,
+        app: Flask | None = None,
+        default_identity: Identity | None = None,
     ) -> None:
         self._default_identity = default_identity
-        self._authenticators: list[Authenticator] = []
-        self._unauthorized_handler: Optional[Callable] = None
-        self.preauth_handler: Optional[PreAuthorizedActionAuthenticator] = None
+        self._authenticators: list[Authenticator] | None = None
+        self._unauthorized_handler: Callable | None = None
+        self.preauth_handler: Authenticator | None = None
 
         if app is not None:
             self.init_app(app)
 
-    def init_app(self, app):
+    def init_app(self, app: Flask) -> None:
         """Initialize the Flask app"""
         app.config.setdefault("AUTH_PROVIDERS", [])
         app.config.setdefault("PRE_AUTHORIZED_ACTION_PROVIDER", None)
 
-    def get_identity(self) -> Optional[Identity]:
+    def get_identity(self) -> Identity | None:
         if hasattr(g, "user") and isinstance(g.user, Identity):
             return g.user
 
@@ -96,19 +98,19 @@ def get_identity(self) -> Optional[Identity]:
         log.debug("No authenticated identity could be found")
         return None
 
-    def login_required(self, f):
+    def login_required(self, f: Callable) -> Callable:
         """A typical Flask "login_required" view decorator"""
 
         @wraps(f)
-        def decorated_function(*args, **kwargs):
+        def decorated_function(*args: Any, **kwargs: Any) -> Any:
             user = self.get_identity()
             if not user:
                 return self.auth_failure()
             return f(*args, **kwargs)
 
         return decorated_function
 
-    def no_identity_handler(self, f):
+    def no_identity_handler(self, f: Callable) -> Callable:
         """Marker decorator for "unauthorized handler" function
 
         This function will be called automatically if no authenticated identity was found
@@ -117,19 +119,19 @@ def no_identity_handler(self, f):
         self._unauthorized_handler = f
 
         @wraps(f)
-        def decorated_func(*args, **kwargs):
+        def decorated_func(*args: Any, **kwargs: Any) -> Any:
             return f(*args, **kwargs)
 
         return decorated_func
 
-    def auth_failure(self):
+    def auth_failure(self) -> Any:
         """Trigger an authentication failure"""
         if self._unauthorized_handler:
             return self._unauthorized_handler()
         else:
             raise Unauthorized("User identity is required")
 
-    def init_authenticators(self, reload=False):
+    def init_authenticators(self, reload: bool = False) -> None:
         """Register an authenticator function"""
         if reload:
             self._authenticators = None
@@ -155,13 +157,18 @@ def init_authenticators(self, reload=False):
             )
             self.push_authenticator(self.preauth_handler)
 
-    def push_authenticator(self, authenticator):
+    def push_authenticator(self, authenticator: Authenticator) -> None:
         """Push an authenticator at the top of the stack"""
+        if self._authenticators is None:
+            self._authenticators = [authenticator]
+            return
         self._authenticators.insert(0, authenticator)
 
-    def _authenticate(self) -> Optional[Identity]:
+    def _authenticate(self) -> Identity | None:
         """Call all registered authenticators until we find an identity"""
         self.init_authenticators()
+        if self._authenticators is None:
+            return self._default_identity
         for authn in self._authenticators:
             try:
                 current_identity = authn(flask_request)