diff --git a/.coveragerc b/.coveragerc deleted file mode 100644 index 658f925..0000000 --- a/.coveragerc +++ /dev/null @@ -1,21 +0,0 @@ -# .coveragerc to control coverage.py - -[run] -branch = True - -[report] -# Regexes for lines to exclude from consideration -exclude_lines = - # Have to re-enable the standard pragma - pragma: no cover - - # We don't care about testing debugging and non-production utility code - def __repr__ - - # Don't complain if tests don't hit defensive assertion code: - raise AssertionError - raise NotImplementedError - - # Don't complain if non-runnable code isn't run: - if 0: - if __name__ == .__main__.: diff --git a/.flake8 b/.flake8 deleted file mode 100644 index 70bdd5b..0000000 --- a/.flake8 +++ /dev/null @@ -1,4 +0,0 @@ -[flake8] -max-line-length = 120 -max-complexity = 10 -exclude = .venv*,venv*,.git,__pycache__,.tox,.eggs,*.egg \ No newline at end of file diff --git a/.isort.cfg b/.isort.cfg deleted file mode 100644 index f77aa42..0000000 --- a/.isort.cfg +++ /dev/null @@ -1,2 +0,0 @@ -[settings] -line_length = 120 diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index dc3a0d3..71bdb1b 100644 --- a/.pre-commit-config.yaml +++ b/.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] diff --git a/.readthedocs.yaml b/.readthedocs.yaml index e034957..13e431c 100644 --- a/.readthedocs.yaml +++ b/.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 diff --git a/.travis.yml b/.travis.yml index d180ce5..886e6b8 100644 --- a/.travis.yml +++ b/.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 diff --git a/Dockerfile b/Dockerfile index 76a89ea..d8ebd0c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -14,7 +14,7 @@ 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 --- @@ -22,7 +22,7 @@ RUN pip wheel -w /wheels -r /requirements.txt 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 diff --git a/MANIFEST.in b/MANIFEST.in index 491e095..bb3ec5f 100644 --- a/MANIFEST.in +++ b/MANIFEST.in @@ -1,2 +1 @@ -include VERSION include README.md diff --git a/Makefile b/Makefile index da0ba28..465233f 100644 --- a/Makefile +++ b/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 diff --git a/VERSION b/VERSION deleted file mode 100644 index 009695b..0000000 --- a/VERSION +++ /dev/null @@ -1,2 +0,0 @@ -0.6.0 - diff --git a/docs/build/.gitignore b/docs/build/.gitignore deleted file mode 100644 index 72e8ffc..0000000 --- a/docs/build/.gitignore +++ /dev/null @@ -1 +0,0 @@ -* diff --git a/docs/source/conf.py b/docs/source/conf.py index 2a116e1..2e1204a 100644 --- a/docs/source/conf.py +++ b/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 --------------------------------------------------- diff --git a/giftless/app.py b/giftless/app.py index 770d2ad..05ede4b 100644 --- a/giftless/app.py +++ b/giftless/app.py @@ -2,9 +2,10 @@ """ 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 @@ -12,7 +13,7 @@ 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__) diff --git a/giftless/auth/__init__.py b/giftless/auth/__init__.py index a969f76..5984148 100644 --- a/giftless/auth/__init__.py +++ b/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,9 +58,9 @@ 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 {} @@ -68,22 +68,24 @@ def get_authz_header( 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,11 +98,11 @@ 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() @@ -108,7 +110,7 @@ def decorated_function(*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) diff --git a/giftless/auth/allow_anon.py b/giftless/auth/allow_anon.py index f1eadcc..7415548 100644 --- a/giftless/auth/allow_anon.py +++ b/giftless/auth/allow_anon.py @@ -13,26 +13,28 @@ want to allow read-only access to anyone), be sure to load this authenticator last. """ +from typing import Any + from .identity import DefaultIdentity, Permission class AnonymousUser(DefaultIdentity): """An anonymous user object""" - def __init__(self, *args, **kwargs): + def __init__(self, *args: Any, **kwargs: Any) -> None: super().__init__(*args, **kwargs) if self.name is None: self.name = "anonymous" -def read_only(_): +def read_only(_: Any) -> AnonymousUser: """Dummy authenticator that gives read-only permissions to everyone""" user = AnonymousUser() user.allow(permissions={Permission.READ, Permission.READ_META}) return user -def read_write(_): +def read_write(_: Any) -> AnonymousUser: """Dummy authenticator that gives full permissions to everyone""" user = AnonymousUser() user.allow(permissions=Permission.all()) diff --git a/giftless/auth/identity.py b/giftless/auth/identity.py index da9eccb..08c42a2 100644 --- a/giftless/auth/identity.py +++ b/giftless/auth/identity.py @@ -42,7 +42,7 @@ def is_authorized( ) -> bool: """Tell if user is authorized to perform an operation on an object / repo""" - def __repr__(self): + def __repr__(self) -> str: return f"<{self.__class__.__name__} id:{self.id} name:{self.name}>" @@ -66,7 +66,7 @@ def allow( repo: Optional[str] = None, permissions: Optional[set[Permission]] = None, oid: Optional[str] = None, - ): + ) -> None: if permissions is None: self._allowed[organization][repo][oid] = set() else: diff --git a/giftless/auth/jwt.py b/giftless/auth/jwt.py index a1a6773..5d9e33c 100644 --- a/giftless/auth/jwt.py +++ b/giftless/auth/jwt.py @@ -126,17 +126,19 @@ def __init__( self._verification_key: Union[str, bytes, None] = None # lazy loaded self._log = logging.getLogger(__name__) - def __call__(self, request: Request) -> Optional[Identity]: + def __call__(self, request: Request) -> Identity | None: token_payload = self._authenticate(request) if token_payload is None: return None return self._get_identity(token_payload) - def get_authz_header(self, *args, **kwargs) -> dict[str, str]: + def get_authz_header(self, *args: Any, **kwargs: Any) -> dict[str, str]: token = self._generate_token_for_action(*args, **kwargs) return {"Authorization": f"Bearer {token}"} - def get_authz_query_params(self, *args, **kwargs) -> dict[str, str]: + def get_authz_query_params( + self, *args: Any, **kwargs: Any + ) -> dict[str, str]: return {"jwt": self._generate_token_for_action(*args, **kwargs)} def _generate_token_for_action( @@ -185,7 +187,7 @@ def _generate_action_scopes( obj_id = f"{org}/{repo}/{oid}" return str(Scope("obj", obj_id, actions)) - def _generate_token(self, **kwargs) -> str: + def _generate_token(self, **kwargs: Any) -> str: """Generate a JWT token that can be used later to authenticate a request""" if not self.private_key: raise ValueError( @@ -220,11 +222,11 @@ def _generate_token(self, **kwargs) -> str: # Type of jwt.encode() went from bytes to str in jwt 2.x, but the # typing hints somehow aren't keeping up. This lets us do the # right thing with jwt 2.x. - if isinstance(token, str): - return token # type: ignore + if isinstance(token, str): # type: ignore[unreachable] + return token # type: ignore[unreachable] return token.decode("ascii") - def _authenticate(self, request: Request): + def _authenticate(self, request: Request) -> dict[str, Any] | None: """Authenticate a request""" token = self._get_token_from_headers(request) if token is None: @@ -253,7 +255,7 @@ def _authenticate(self, request: Request): f"Expired or otherwise invalid JWT token ({e!s})" ) - def _get_token_from_headers(self, request: Request) -> Optional[str]: + def _get_token_from_headers(self, request: Request) -> str | None: """Extract JWT token from HTTP Authorization header This will first try to obtain a Bearer token. If none is found but we have a 'Basic' Authorization header, @@ -375,34 +377,29 @@ def _get_verification_key(self) -> Union[str, bytes]: class Scope: """Scope object""" - entity_type = None - subscope = None - entity_ref = None - actions = None - def __init__( self, entity_type: str, - entity_id: Optional[str] = None, - actions: Optional[set[str]] = None, - subscope: Optional[str] = None, - ): + entity_id: str | None = None, + actions: set[str] | None = None, + subscope: str | None = None, + ) -> None: self.entity_type = entity_type self.entity_ref = entity_id self.actions = actions self.subscope = subscope - def __repr__(self): + def __repr__(self) -> str: return f"" - def __str__(self): + def __str__(self) -> str: """Convert scope to a string""" parts = [self.entity_type] entity_ref = self.entity_ref if self.entity_ref != "*" else None subscobe = self.subscope if self.subscope != "*" else None actions = ( ",".join(sorted(self.actions)) - if self.actions and self.actions != "*" + if self.actions and self.actions != set("*") else None ) @@ -422,7 +419,7 @@ def __str__(self): return ":".join(parts) @classmethod - def from_string(cls, scope_str): + def from_string(cls, scope_str: str) -> "Scope": """Create a scope object from string""" parts = scope_str.split(":") if len(parts) < 1: @@ -447,7 +444,7 @@ def _parse_actions(cls, actions_str: str) -> set[str]: return set(actions_str.split(",")) -def factory(**options): +def factory(**options: Any) -> JWTAuthenticator: for key_type in ("private_key", "public_key"): file_opt = f"{key_type}_file" try: diff --git a/giftless/config.py b/giftless/config.py index 0ea7d77..66bddd7 100644 --- a/giftless/config.py +++ b/giftless/config.py @@ -1,25 +1,24 @@ """Configuration handling helper functions and default configuration """ import os -from typing import Optional +from typing import Any -import figcan import yaml from dotenv import load_dotenv +from figcan import Configuration, Extensible # type:ignore +from flask import Flask ENV_PREFIX = "GIFTLESS_" ENV_FILE = ".env" default_transfer_config = { - "basic": figcan.Extensible( + "basic": Extensible( { "factory": "giftless.transfer.basic_streaming:factory", - "options": figcan.Extensible( + "options": Extensible( { "storage_class": "giftless.storage.local_storage:LocalStorage", - "storage_options": figcan.Extensible( - {"path": "lfs-storage"} - ), + "storage_options": Extensible({"path": "lfs-storage"}), "action_lifetime": 900, } ), @@ -28,7 +27,7 @@ } default_config = { - "TRANSFER_ADAPTERS": figcan.Extensible(default_transfer_config), + "TRANSFER_ADAPTERS": Extensible(default_transfer_config), "TESTING": False, "DEBUG": False, "AUTH_PROVIDERS": ["giftless.auth.allow_anon:read_only"], @@ -50,7 +49,7 @@ load_dotenv() -def configure(app, additional_config: Optional[dict] = None): +def configure(app: Flask, additional_config: dict | None = None) -> Flask: """Configure a Flask app using Figcan managed configuration object""" config = _compose_config(additional_config) app.config.update(config) @@ -58,10 +57,10 @@ def configure(app, additional_config: Optional[dict] = None): def _compose_config( - additional_config: Optional[dict] = None, -) -> figcan.Configuration: + additional_config: dict[str, Any] | None = None, +) -> Configuration: """Compose configuration object from all available sources""" - config = figcan.Configuration(default_config) + config = Configuration(default_config) environ = dict( os.environ ) # Copy the environment as we're going to change it @@ -77,7 +76,7 @@ def _compose_config( config.apply(config_from_file) environ.pop(f"{ENV_PREFIX}CONFIG_STR") - config.apply_flat(environ, prefix=ENV_PREFIX) # type: ignore + config.apply_flat(environ, prefix=ENV_PREFIX) if additional_config: config.apply(additional_config) diff --git a/giftless/error_handling.py b/giftless/error_handling.py index ed99bc6..c9852ad 100644 --- a/giftless/error_handling.py +++ b/giftless/error_handling.py @@ -2,22 +2,23 @@ See https://github.com/git-lfs/git-lfs/blob/master/docs/api/batch.md#response-errors """ +from flask import Flask, Response from werkzeug.exceptions import default_exceptions from .representation import output_git_lfs_json class ApiErrorHandler: - def __init__(self, app=None): + def __init__(self, app: Flask | None = None) -> None: if app: self.init_app(app) - def init_app(self, app): + def init_app(self, app: Flask) -> None: for code in default_exceptions: app.errorhandler(code)(self.error_as_json) @classmethod - def error_as_json(cls, ex): + def error_as_json(cls, ex: Exception) -> Response: """Handle errors by returning a JSON response""" code = ex.code if hasattr(ex, "code") else 500 data = {"message": str(ex)} diff --git a/giftless/representation.py b/giftless/representation.py index 1b89771..4f21c59 100644 --- a/giftless/representation.py +++ b/giftless/representation.py @@ -8,8 +8,9 @@ import json from datetime import datetime from functools import partial +from typing import Any -from flask import make_response +from flask import Response, make_response GIT_LFS_MIME_TYPE = "application/vnd.git-lfs+json" @@ -17,13 +18,18 @@ class CustomJsonEncoder(json.JSONEncoder): """Custom JSON encoder that can support some additional required types""" - def default(self, o): + def default(self, o: Any) -> Any: if isinstance(o, datetime): return o.isoformat() return super().default(o) -def output_json(data, code, headers=None, content_type="application/json"): +def output_json( + data: Any, + code: int | None, + headers: dict[str, str] | None = None, + content_type: str = "application/json", +) -> Response: dumped = json.dumps(data, cls=CustomJsonEncoder) if headers: headers.update({"Content-Type": content_type}) diff --git a/giftless/schema.py b/giftless/schema.py index 0a22c7e..ea09c91 100644 --- a/giftless/schema.py +++ b/giftless/schema.py @@ -1,9 +1,10 @@ """Schema for Git LFS APIs """ from enum import Enum +from typing import Any import marshmallow -from flask_marshmallow import Marshmallow # type: ignore +from flask_marshmallow import Marshmallow from marshmallow import fields, pre_load, validate from marshmallow_enum import EnumField @@ -32,7 +33,9 @@ class ObjectSchema(ma.Schema): # type: ignore extra = fields.Dict(required=False, missing=dict) @pre_load - def set_extra_fields(self, data, **_): + def set_extra_fields( + self, data: dict[str, Any], **_: Any + ) -> dict[str, Any]: extra = {} rest = {} for k, v in data.items(): diff --git a/giftless/storage/__init__.py b/giftless/storage/__init__.py index 237664a..f91afa3 100644 --- a/giftless/storage/__init__.py +++ b/giftless/storage/__init__.py @@ -39,10 +39,10 @@ def exists(self, prefix: str, oid: str) -> bool: def get_size(self, prefix: str, oid: str) -> int: pass - def get_mime_type(self, prefix: str, oid: str) -> Optional[str]: + def get_mime_type(self, prefix: str, oid: str) -> str: return "application/octet-stream" - def verify_object(self, prefix: str, oid: str, size: int): + def verify_object(self, prefix: str, oid: str, size: int) -> bool: """Verify that an object exists""" try: return self.get_size(prefix, oid) == size @@ -60,7 +60,7 @@ def get_upload_action( oid: str, size: int, expires_in: int, - extra: Optional[dict[str, Any]] = None, + extra: dict[str, Any] | None = None, ) -> dict[str, Any]: pass @@ -71,7 +71,7 @@ def get_download_action( oid: str, size: int, expires_in: int, - extra: Optional[dict[str, Any]] = None, + extra: dict[str, Any] | None = None, ) -> dict[str, Any]: pass @@ -99,7 +99,7 @@ def get_multipart_actions( size: int, part_size: int, expires_in: int, - extra: Optional[dict[str, Any]] = None, + extra: dict[str, Any] | None = None, ) -> dict[str, Any]: pass @@ -110,7 +110,7 @@ def get_download_action( oid: str, size: int, expires_in: int, - extra: Optional[dict[str, Any]] = None, + extra: dict[str, Any] | None = None, ) -> dict[str, Any]: pass @@ -129,5 +129,5 @@ def verify_object(self, prefix: str, oid: str, size: int) -> bool: return False -def guess_mime_type_from_filename(filename: str) -> Optional[str]: +def guess_mime_type_from_filename(filename: str) -> str | None: return mimetypes.guess_type(filename)[0] diff --git a/giftless/storage/amazon_s3.py b/giftless/storage/amazon_s3.py index 0c1999e..f96df6a 100644 --- a/giftless/storage/amazon_s3.py +++ b/giftless/storage/amazon_s3.py @@ -2,7 +2,7 @@ import binascii import posixpath from collections.abc import Iterable -from typing import Any, BinaryIO, Optional +from typing import Any, BinaryIO import boto3 import botocore @@ -18,10 +18,10 @@ class AmazonS3Storage(StreamingStorage, ExternalStorage): def __init__( self, bucket_name: str, - path_prefix: Optional[str] = None, - endpoint: Optional[str] = None, - **_, - ): + path_prefix: str | None = None, + endpoint: str | None = None, + **_: Any, + ) -> None: self.bucket_name = bucket_name self.path_prefix = path_prefix self.s3 = boto3.resource("s3", endpoint_url=endpoint) @@ -34,9 +34,9 @@ def get(self, prefix: str, oid: str) -> Iterable[bytes]: return result def put(self, prefix: str, oid: str, data_stream: BinaryIO) -> int: - completed = [] + completed: list[int] = [] - def upload_callback(size): + def upload_callback(size: int) -> None: completed.append(size) bucket = self.s3.Bucket(self.bucket_name) @@ -70,7 +70,7 @@ def get_upload_action( oid: str, size: int, expires_in: int, - extra: Optional[dict[str, Any]] = None, + extra: dict[str, Any] | None = None, ) -> dict[str, Any]: base64_oid = base64.b64encode(binascii.a2b_hex(oid)).decode("ascii") params = { @@ -101,7 +101,7 @@ def get_download_action( oid: str, size: int, expires_in: int, - extra: Optional[dict[str, str]] = None, + extra: dict[str, str] | None = None, ) -> dict[str, Any]: params = { "Bucket": self.bucket_name, @@ -144,7 +144,7 @@ def _get_blob_path(self, prefix: str, oid: str) -> str: storage_prefix = self.path_prefix return posixpath.join(storage_prefix, prefix, oid) - def _s3_object(self, prefix, oid): + def _s3_object(self, prefix: str, oid: str) -> Any: return self.s3.Object( self.bucket_name, self._get_blob_path(prefix, oid) ) diff --git a/giftless/storage/azure.py b/giftless/storage/azure.py index 4979e34..ca0a84d 100644 --- a/giftless/storage/azure.py +++ b/giftless/storage/azure.py @@ -16,7 +16,6 @@ generate_blob_sas, ) -# type: ignore from giftless.storage import ( ExternalStorage, MultipartStorage, @@ -44,8 +43,8 @@ def __init__( container_name: str, path_prefix: Optional[str] = None, enable_content_digest: bool = True, - **_, - ): + **_: Any, + ) -> None: self.container_name = container_name self.path_prefix = path_prefix self.blob_svc_client: BlobServiceClient = ( @@ -59,7 +58,7 @@ def get(self, prefix: str, oid: str) -> Iterable[bytes]: blob=self._get_blob_path(prefix, oid), ) try: - return blob_client.download_blob().chunks() # type: ignore + return blob_client.download_blob().chunks() except ResourceNotFoundError: raise ObjectNotFound("Object does not exist") @@ -68,7 +67,7 @@ def put(self, prefix: str, oid: str, data_stream: IO[bytes]) -> int: container=self.container_name, blob=self._get_blob_path(prefix, oid), ) - blob_client.upload_blob(data_stream) # type: ignore + blob_client.upload_blob(data_stream) return data_stream.tell() def exists(self, prefix: str, oid: str) -> bool: @@ -85,11 +84,11 @@ def get_size(self, prefix: str, oid: str) -> int: blob=self._get_blob_path(prefix, oid), ) props = blob_client.get_blob_properties() - return props.size # type: ignore + return props.size except ResourceNotFoundError: raise ObjectNotFound("Object does not exist") - def get_mime_type(self, prefix: str, oid: str) -> Optional[str]: + def get_mime_type(self, prefix: str, oid: str) -> str: try: blob_client = self.blob_svc_client.get_blob_client( container=self.container_name, @@ -99,7 +98,9 @@ def get_mime_type(self, prefix: str, oid: str) -> Optional[str]: mime_type = props.content_settings.get( "content_type", "application/octet-stream" ) - return mime_type # type: ignore + if mime_type is None: + return "application/octet-stream" + return str(mime_type) except ResourceNotFoundError: raise ObjectNotFound("Object does not exist") @@ -386,7 +387,7 @@ def _calculate_blocks(file_size: int, part_size: int) -> list[Block]: >>> _calculate_blocks(0, 10) [] - """ + """ # noqa: E501 full_blocks = file_size // part_size last_block_size = file_size % part_size blocks = [ diff --git a/giftless/storage/exc.py b/giftless/storage/exc.py index eca1453..ef807cb 100644 --- a/giftless/storage/exc.py +++ b/giftless/storage/exc.py @@ -1,14 +1,13 @@ """Storage related errors """ -from typing import Optional class StorageError(RuntimeError): """Base class for storage errors""" - code: Optional[int] = None + code: int | None = None - def as_dict(self): + def as_dict(self) -> dict[str, str | int | None]: return {"message": str(self), "code": self.code} diff --git a/giftless/storage/google_cloud.py b/giftless/storage/google_cloud.py index 939db43..5c55889 100644 --- a/giftless/storage/google_cloud.py +++ b/giftless/storage/google_cloud.py @@ -3,12 +3,12 @@ import json import posixpath from datetime import timedelta -from typing import Any, BinaryIO, Optional, Union +from typing import Any, BinaryIO, Union import google.auth from google.auth import impersonated_credentials -from google.cloud import storage # type: ignore -from google.oauth2 import service_account # type: ignore +from google.cloud import storage +from google.oauth2 import service_account from giftless.storage import ExternalStorage, StreamingStorage @@ -24,19 +24,18 @@ def __init__( self, project_name: str, bucket_name: str, - account_key_file: Optional[str] = None, - account_key_base64: Optional[str] = None, - path_prefix: Optional[str] = None, - serviceaccount_email: Optional[str] = None, - **_, - ): + account_key_file: str | None = None, + account_key_base64: str | None = None, + path_prefix: str | None = None, + serviceaccount_email: str | None = None, + **_: Any, + ) -> None: self.bucket_name = bucket_name self.path_prefix = path_prefix - self.credentials: Optional[ - Union[ - service_account.Credentials, - impersonated_credentials.Credentials, - ] + self.credentials: Union[ + service_account.Credentials, + impersonated_credentials.Credentials, + None, ] = self._load_credentials(account_key_file, account_key_base64) self.storage_client = storage.Client( project=project_name, credentials=self.credentials @@ -83,7 +82,7 @@ def get_upload_action( oid: str, size: int, expires_in: int, - extra: Optional[dict[str, Any]] = None, + extra: dict[str, Any] | None = None, ) -> dict[str, Any]: return { "actions": { @@ -103,7 +102,7 @@ def get_download_action( oid: str, size: int, expires_in: int, - extra: Optional[dict[str, Any]] = None, + extra: dict[str, Any] | None = None, ) -> dict[str, Any]: filename = extra.get("filename") if extra else None disposition = ( @@ -142,8 +141,8 @@ def _get_signed_url( oid: str, expires_in: int, http_method: str = "GET", - filename: Optional[str] = None, - disposition: Optional[str] = None, + filename: str | None = None, + disposition: str | None = None, ) -> str: creds = self.credentials if creds is None: @@ -165,8 +164,8 @@ def _get_signed_url( @staticmethod def _load_credentials( - account_key_file: Optional[str], account_key_base64: Optional[str] - ) -> Optional[service_account.Credentials]: + account_key_file: str | None, account_key_base64: str | None + ) -> service_account.Credentials | None: """Load Google Cloud credentials from passed configuration""" if account_key_file and account_key_base64: raise ValueError( diff --git a/giftless/storage/local_storage.py b/giftless/storage/local_storage.py index 06c92d5..f602f85 100644 --- a/giftless/storage/local_storage.py +++ b/giftless/storage/local_storage.py @@ -2,6 +2,8 @@ import shutil from typing import Any, BinaryIO, Optional +from flask import Flask + from giftless.storage import MultipartStorage, StreamingStorage, exc from giftless.view import ViewProvider @@ -14,7 +16,7 @@ class LocalStorage(StreamingStorage, MultipartStorage, ViewProvider): want to use a more scalable solution such as one of the cloud storage backends. """ - def __init__(self, path: Optional[str] = None, **_) -> None: + def __init__(self, path: Optional[str] = None, **_: Any) -> None: if path is None: path = "lfs-storage" self.path = path @@ -69,13 +71,13 @@ def get_download_action( ) -> dict[str, Any]: return {} - def register_views(self, app): + def register_views(self, app: Flask) -> None: super().register_views(app) def _get_path(self, prefix: str, oid: str) -> str: return os.path.join(self.path, prefix, oid) @staticmethod - def _create_path(path): + def _create_path(path: str) -> None: if not os.path.isdir(path): os.makedirs(path) diff --git a/giftless/transfer/__init__.py b/giftless/transfer/__init__.py index 8533d16..0bfe0e0 100644 --- a/giftless/transfer/__init__.py +++ b/giftless/transfer/__init__.py @@ -3,12 +3,18 @@ See https://github.com/git-lfs/git-lfs/blob/master/docs/api/basic-transfers.md for more information about what transfer APIs do in Git LFS. """ -from abc import ABC +from abc import ABC, abstractmethod from collections.abc import Callable from functools import partial -from typing import Any, Optional +from typing import Any, Optional, cast -from giftless.auth import Authentication, authentication +from flask import Flask + +from giftless.auth import ( + Authentication, + PreAuthorizedActionAuthenticator, + authentication, +) from giftless.util import add_query_params, get_callable from giftless.view import ViewProvider @@ -24,7 +30,7 @@ def upload( repo: str, oid: str, size: int, - extra: Optional[dict[str, Any]] = None, + extra: dict[str, Any] | None = None, ) -> dict: raise NotImplementedError( "This transfer adapter is not fully implemented" @@ -36,7 +42,7 @@ def download( repo: str, oid: str, size: int, - extra: Optional[dict[str, Any]] = None, + extra: dict[str, Any] | None = None, ) -> dict: raise NotImplementedError( "This transfer adapter is not fully implemented" @@ -52,14 +58,19 @@ def get_action( class PreAuthorizingTransferAdapter(TransferAdapter, ABC): - """A transfer adapter that can pre-authohrize one or more of the actions it supports""" - - # Lifetime of verify tokens can be very long - VERIFY_LIFETIME = 3600 * 12 - - _auth_module: Optional[Authentication] = None - - def set_auth_module(self, auth_module: Authentication): + """A transfer adapter that can pre-authorize one or more of the actions it supports""" + + @abstractmethod + def __init__(self) -> None: + # + # These were class attributes, but at least _auth_module ought to + # be an instance attribute instead. + # + self.VERIFY_LIFETIME = 12 * 60 * 60 # Can be quite a while + if not hasattr(self, "_auth_module"): + self._auth_module: Authentication | None = None + + def set_auth_module(self, auth_module: Authentication) -> None: self._auth_module = auth_module def _preauth_url( @@ -67,18 +78,23 @@ def _preauth_url( original_url: str, 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, ) -> str: - if not (self._auth_module and self._auth_module.preauth_handler): + if self._auth_module is None: + return original_url + if self._auth_module.preauth_handler is None: return original_url + handler = cast( + PreAuthorizedActionAuthenticator, self._auth_module.preauth_handler + ) identity = self._auth_module.get_identity() if identity is None: return original_url - params = self._auth_module.preauth_handler.get_authz_query_params( + params = handler.get_authz_query_params( identity, org, repo, actions, oid, lifetime=lifetime ) @@ -88,23 +104,28 @@ def _preauth_headers( self, 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]: - if not (self._auth_module and self._auth_module.preauth_handler): + if self._auth_module is None: + return {} + if self._auth_module.preauth_handler is None: return {} + handler = cast( + PreAuthorizedActionAuthenticator, self._auth_module.preauth_handler + ) + identity = self._auth_module.get_identity() if identity is None: return {} - - return self._auth_module.preauth_handler.get_authz_header( + return handler.get_authz_header( identity, org, repo, actions, oid, lifetime=lifetime ) -def init_flask_app(app): +def init_flask_app(app: Flask) -> None: """Initialize a flask app instance with transfer adapters. This will: @@ -122,7 +143,7 @@ def init_flask_app(app): adapter.register_views(app) -def register_adapter(key: str, adapter: TransferAdapter): +def register_adapter(key: str, adapter: TransferAdapter) -> None: """Register a transfer adapter""" _registered_adapters[key] = adapter diff --git a/giftless/transfer/basic_external.py b/giftless/transfer/basic_external.py index 87c26a2..cdeeaba 100644 --- a/giftless/transfer/basic_external.py +++ b/giftless/transfer/basic_external.py @@ -8,22 +8,26 @@ downloads, this transfer adapter can work with them. Different storage backends can be used with this adapter, as long as they -implement the `ExternalStorage` interface defined here. +implement the `ExternalStorage` interface defined in giftless.storage. """ import posixpath from typing import Any, Optional +from flask import Flask + from giftless.storage import ExternalStorage, exc -from giftless.transfer import PreAuthorizingTransferAdapter, ViewProvider +from giftless.transfer import PreAuthorizingTransferAdapter from giftless.transfer.basic_streaming import VerifyView from giftless.util import get_callable +from giftless.view import ViewProvider class BasicExternalBackendTransferAdapter( PreAuthorizingTransferAdapter, ViewProvider ): def __init__(self, storage: ExternalStorage, default_action_lifetime: int): + super().__init__() self.storage = storage self.action_lifetime = default_action_lifetime @@ -90,10 +94,10 @@ def download( return response - def register_views(self, app): + def register_views(self, app: Flask) -> None: VerifyView.register(app, init_argument=self.storage) - def _check_object(self, prefix: str, oid: str, size: int): + def _check_object(self, prefix: str, oid: str, size: int) -> None: """Raise specific domain error if object is not valid NOTE: this does not use storage.verify_object directly because @@ -103,7 +107,9 @@ def _check_object(self, prefix: str, oid: str, size: int): raise exc.InvalidObject("Object size does not match") -def factory(storage_class, storage_options, action_lifetime): +def factory( + storage_class: Any, storage_options: Any, action_lifetime: int +) -> BasicExternalBackendTransferAdapter: """Factory for basic transfer adapter with external storage""" storage = get_callable(storage_class, __name__) return BasicExternalBackendTransferAdapter( diff --git a/giftless/transfer/basic_streaming.py b/giftless/transfer/basic_streaming.py index 75c2f71..82ce3bf 100644 --- a/giftless/transfer/basic_streaming.py +++ b/giftless/transfer/basic_streaming.py @@ -7,20 +7,20 @@ """ import posixpath -from typing import Any, Optional +from typing import Any, BinaryIO, Optional, cast import marshmallow -from flask import Response, request, url_for +from flask import Flask, Response, request, url_for from flask_classful import route -from webargs.flaskparser import parser # type: ignore +from webargs.flaskparser import parser from giftless.auth.identity import Permission from giftless.exc import InvalidPayload, NotFound from giftless.schema import ObjectSchema from giftless.storage import StreamingStorage, VerifiableStorage -from giftless.transfer import PreAuthorizingTransferAdapter, ViewProvider +from giftless.transfer import PreAuthorizingTransferAdapter from giftless.util import add_query_params, get_callable, safe_filename -from giftless.view import BaseView +from giftless.view import BaseView, ViewProvider class VerifyView(BaseView): @@ -36,7 +36,7 @@ def __init__(self, storage: VerifiableStorage): self.storage = storage @route("/verify", methods=["POST"]) - def verify(self, organization, repo): + def verify(self, organization: str, repo: str) -> Response: schema = ObjectSchema(unknown=marshmallow.EXCLUDE) payload = parser.parse(schema) @@ -75,7 +75,7 @@ class ObjectsView(BaseView): def __init__(self, storage: StreamingStorage): self.storage = storage - def put(self, organization, repo, oid): + def put(self, organization: str, repo: str, oid: str) -> Response: """Upload a file to local storage For now, I am not sure this actually streams chunked uploads without reading the entire @@ -88,11 +88,13 @@ def put(self, organization, repo, oid): ) stream = request.stream self.storage.put( - prefix=f"{organization}/{repo}", oid=oid, data_stream=stream + prefix=f"{organization}/{repo}", + oid=oid, + data_stream=cast(BinaryIO, stream), ) return Response(status=200) - def get(self, organization, repo, oid): + def get(self, organization: str, repo: str, oid: str) -> Response: """Get an file open file stream from local storage""" self._check_authorization(organization, repo, Permission.READ, oid=oid) path = posixpath.join(organization, repo) @@ -143,6 +145,7 @@ class BasicStreamingTransferAdapter( PreAuthorizingTransferAdapter, ViewProvider ): def __init__(self, storage: StreamingStorage, action_lifetime: int): + super().__init__() self.storage = storage self.action_lifetime = action_lifetime @@ -233,12 +236,14 @@ def download( return response - def register_views(self, app): + def register_views(self, app: Flask) -> None: ObjectsView.register(app, init_argument=self.storage) VerifyView.register(app, init_argument=self.storage) -def factory(storage_class, storage_options, action_lifetime): +def factory( + storage_class: Any, storage_options: Any, action_lifetime: int +) -> BasicStreamingTransferAdapter: """Factory for basic transfer adapter with local storage""" storage = get_callable(storage_class, __name__) return BasicStreamingTransferAdapter( diff --git a/giftless/transfer/multipart.py b/giftless/transfer/multipart.py index 9b71769..a81e983 100644 --- a/giftless/transfer/multipart.py +++ b/giftless/transfer/multipart.py @@ -4,10 +4,13 @@ import posixpath from typing import Any, Optional +from flask import Flask + from giftless.storage import MultipartStorage, exc -from giftless.transfer import PreAuthorizingTransferAdapter, ViewProvider +from giftless.transfer import PreAuthorizingTransferAdapter from giftless.transfer.basic_streaming import VerifyView from giftless.util import get_callable +from giftless.view import ViewProvider DEFAULT_PART_SIZE = 10240000 # 10mb DEFAULT_ACTION_LIFETIME = 6 * 3600 # 6 hours @@ -20,6 +23,7 @@ def __init__( default_action_lifetime: int, max_part_size: int = DEFAULT_PART_SIZE, ): + super().__init__() self.storage = storage self.max_part_size = max_part_size self.action_lifetime = default_action_lifetime @@ -86,13 +90,13 @@ def download( return response - def register_views(self, app): + def register_views(self, app: Flask) -> None: # FIXME: this is broken. Need to find a smarter way for multiple transfer adapters to provide the same view # VerifyView.register(app, init_argument=self.storage) if isinstance(self.storage, ViewProvider): self.storage.register_views(app) - def _check_object(self, prefix: str, oid: str, size: int): + def _check_object(self, prefix: str, oid: str, size: int) -> None: """Raise specific domain error if object is not valid NOTE: this does not use storage.verify_object directly because @@ -103,11 +107,11 @@ def _check_object(self, prefix: str, oid: str, size: int): def factory( - storage_class, - storage_options, + storage_class: Any, + storage_options: Any, action_lifetime: int = DEFAULT_ACTION_LIFETIME, max_part_size: int = DEFAULT_PART_SIZE, -): +) -> MultipartTransferAdapter: """Factory for multipart transfer adapter with storage""" try: storage = get_callable(storage_class, __name__) diff --git a/giftless/util.py b/giftless/util.py index 1ada902..bc50cbb 100644 --- a/giftless/util.py +++ b/giftless/util.py @@ -69,7 +69,7 @@ def add_query_params(url: str, params: dict[str, Any]) -> str: >>> add_query_params('https://example.org?param1=value1', {'param2': 'value2'}) 'https://example.org?param1=value1¶m2=value2' - """ + """ # noqa: E501 urlencoded_params = urlencode(params) separator = "&" if "?" in url else "?" return f"{url}{separator}{urlencoded_params}" diff --git a/giftless/view.py b/giftless/view.py index 8bb0d9f..70989a2 100644 --- a/giftless/view.py +++ b/giftless/view.py @@ -1,9 +1,10 @@ """Flask-Classful View Classes """ -from typing import Any, Optional +from typing import Any +from flask import Flask, Response from flask_classful import FlaskView -from webargs.flaskparser import parser # type: ignore +from webargs.flaskparser import parser from giftless import exc, representation, schema, transfer from giftless.auth import authentication as authn @@ -26,13 +27,19 @@ class BaseView(FlaskView): trailing_slash = False @classmethod - def register(cls, *args, **kwargs): + def register(cls, *args: Any, **kwargs: Any) -> Any: if kwargs.get("base_class") is None: kwargs["base_class"] = BaseView return super().register(*args, **kwargs) @classmethod - def _check_authorization(cls, organization, repo, permission, oid=None): + def _check_authorization( + cls, + organization: str, + repo: str, + permission: Permission, + oid: str | None = None, + ) -> None: """Check the current user is authorized to perform an action and raise an exception otherwise""" if not cls._is_authorized(organization, repo, permission, oid): raise exc.Forbidden( @@ -40,10 +47,15 @@ def _check_authorization(cls, organization, repo, permission, oid=None): ) @staticmethod - def _is_authorized(organization, repo, permission, oid=None): + def _is_authorized( + organization: str, + repo: str, + permission: Permission, + oid: str | None = None, + ) -> bool: """Check the current user is authorized to perform an action""" identity = authn.get_identity() - return identity and identity.is_authorized( + return identity is not None and identity.is_authorized( organization, repo, permission, oid ) @@ -53,7 +65,7 @@ class BatchView(BaseView): route_base = "//objects/batch" - def post(self, organization, repo): + def post(self, organization: str, repo: str) -> dict[str, Any]: """Batch operations""" payload = parser.parse(schema.batch_request_schema) @@ -62,7 +74,7 @@ def post(self, organization, repo): payload["transfers"] ) except ValueError as e: - raise exc.InvalidPayload(e) + raise exc.InvalidPayload(str(e)) permission = ( Permission.WRITE @@ -79,11 +91,11 @@ def post(self, organization, repo): ): raise - response = {"transfer": transfer_type} + response: dict[str, Any] = {"transfer": transfer_type} action = adapter.get_action( payload["operation"].value, organization, repo ) - response["objects"] = [action(**o) for o in payload["objects"]] + response["objects"] = [action(**o) for o in payload["objects"]] # type: ignore[call-arg] if all(self._is_error(o, 404) for o in response["objects"]): raise exc.NotFound("Cannot find any of the requested objects") @@ -99,7 +111,7 @@ def post(self, organization, repo): return response @staticmethod - def _is_error(obj: dict[str, Any], code: Optional[int] = None): + def _is_error(obj: dict[str, Any], code: int | None = None) -> bool: try: return obj["error"]["code"] == code or code is None except KeyError: @@ -113,5 +125,5 @@ class ViewProvider: directly from the Giftless HTTP server. """ - def register_views(self, app): + def register_views(self, app: Flask) -> None: pass diff --git a/mypy.ini b/mypy.ini deleted file mode 100644 index 6605600..0000000 --- a/mypy.ini +++ /dev/null @@ -1,13 +0,0 @@ -[mypy] -warn_return_any = True -warn_unused_configs = True -namespace_packages = True - -[mypy-flask_classful] -ignore_missing_imports = True - -[mypy-marshmallow_enum] -ignore_missing_imports = True - -[mypy-webargs] -ignore_missing_imports = True diff --git a/pyproject.toml b/pyproject.toml index 9933044..8dd4e10 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -98,17 +98,24 @@ disallow_untyped_defs = true disallow_incomplete_defs = true ignore_missing_imports = true local_partial_types = true +namespace_packages = true no_implicit_reexport = true -plugins = [ - "pydantic.mypy", - "sqlalchemy.ext.mypy.plugin", -] show_error_codes = true strict_equality = true warn_redundant_casts = true +warn_return_any = true warn_unreachable = true +warn_unused_configs = true warn_unused_ignores = true +[[tool.mypy.overrides]] +module = [ + "flask_classful", + "marshmallow_enum", + "webargs" +] +ignore_missing_imports = true + [tool.pydantic-mypy] init_forbid_extra = true init_typed = true @@ -116,7 +123,19 @@ warn_required_dynamic_aliases = true warn_untyped_fields = true [tool.pytest.ini_options] -asyncio_mode = "strict" +testpaths = [ + "tests" +] +env = [ + "D:AZURE_CONNECTION_STRING=", + "D:AZURE_CONTAINER=", + "D:GCP_BUCKET_NAME=", + "D:GCP_PROJECT_NAME=", + "D:GCP_ACCOUNT_KEY_FILE=", +] +addopts = [ + "--doctest-modules" +] filterwarnings = [ # Google modules call a deprecated pkg_resources API. "ignore:pkg_resources is deprecated as an API:DeprecationWarning", @@ -207,7 +226,7 @@ ignore = [ "ISC002", ] select = ["ALL"] -target-version = "py311" +target-version = "py310" [tool.ruff.per-file-ignores] "tests/**" = [ diff --git a/pytest.ini b/pytest.ini deleted file mode 100644 index 17f0948..0000000 --- a/pytest.ini +++ /dev/null @@ -1,10 +0,0 @@ -[pytest] -addopts = --mypy --doctest-modules -testpaths = - tests -env = - D:AZURE_CONNECTION_STRING= - D:AZURE_CONTAINER= - D:GCP_BUCKET_NAME= - D:GCP_PROJECT_NAME= - D:GCP_ACCOUNT_KEY_FILE= diff --git a/dev-requirements.in b/requirements/dev.in similarity index 62% rename from dev-requirements.in rename to requirements/dev.in index 0569177..f8305f0 100644 --- a/dev-requirements.in +++ b/requirements/dev.in @@ -1,4 +1,4 @@ --c requirements.txt +-c main.txt pip-tools tox @@ -23,6 +23,7 @@ google-auth-stubs # Documentation Requirements recommonmark furo -# https://github.com/PyCQA/flake8/pull/1438 - see requirements.in -sphinx<4.4.0 -sphinx-autodoc-typehints[type_comments]<1.18.0 +sphinx +sphinx-autodoc-typehints + + diff --git a/dev-requirements.txt b/requirements/dev.txt similarity index 58% rename from dev-requirements.txt rename to requirements/dev.txt index 03827e9..0768048 100644 --- a/dev-requirements.txt +++ b/requirements/dev.txt @@ -1,84 +1,80 @@ # -# This file is autogenerated by pip-compile with Python 3.10 +# This file is autogenerated by pip-compile with Python 3.11 # by the following command: # -# pip-compile --no-emit-index-url --output-file=dev-requirements.txt dev-requirements.in +# pip-compile --no-emit-index-url --output-file=requirements/dev.txt requirements/dev.in # -alabaster==0.7.13 +alabaster==0.7.16 # via sphinx -attrs==23.1.0 +attrs==23.2.0 # via pytest-mypy babel==2.14.0 # via sphinx beautifulsoup4==4.12.2 # via furo -boto3-stubs==1.34.5 - # via -r dev-requirements.in -botocore-stubs==1.34.5 +boto3-stubs==1.34.16 + # via -r requirements/dev.in +botocore-stubs==1.34.16 # via - # -r dev-requirements.in + # -r requirements/dev.in # boto3-stubs build==1.0.3 # via pip-tools cachetools==5.3.2 # via - # -c requirements.txt + # -c requirements/main.txt # google-auth # tox certifi==2023.11.17 # via - # -c requirements.txt + # -c requirements/main.txt # requests chardet==5.2.0 # via tox charset-normalizer==3.3.2 # via - # -c requirements.txt + # -c requirements/main.txt # requests click==8.1.7 # via - # -c requirements.txt + # -c requirements/main.txt # pip-tools colorama==0.4.6 # via tox commonmark==0.9.1 # via recommonmark -coverage[toml]==7.3.4 +coverage[toml]==7.4.0 # via # coverage # pytest-cov distlib==0.3.8 # via virtualenv -docutils==0.17.1 +docutils==0.20.1 # via # recommonmark # sphinx -exceptiongroup==1.2.0 - # via - # -c requirements.txt - # pytest filelock==3.13.1 # via # pytest-mypy # tox # virtualenv -flake8==6.1.0 - # via -r dev-requirements.in -furo==2022.9.29 - # via -r dev-requirements.in -google-auth==2.25.2 +flake8==7.0.0 + # via -r requirements/dev.in +furo==2023.9.10 + # via -r requirements/dev.in +google-auth==2.26.1 # via - # -c requirements.txt + # -c requirements/main.txt # google-auth-stubs google-auth-stubs==0.2.0 - # via -r dev-requirements.in -grpc-stubs==1.53.0.3 + # via -r requirements/dev.in +grpc-stubs==1.53.0.5 # via google-auth-stubs grpcio==1.60.0 # via grpc-stubs idna==3.6 # via - # -c requirements.txt + # -c requirements/main.txt # requests # yarl imagesize==1.4.1 @@ -87,32 +83,32 @@ iniconfig==2.0.0 # via pytest isort==5.13.2 # via pytest-isort -jinja2==3.1.2 +jinja2==3.1.3 # via - # -c requirements.txt + # -c requirements/main.txt # sphinx markupsafe==2.1.3 # via - # -c requirements.txt + # -c requirements/main.txt # jinja2 mccabe==0.7.0 # via flake8 multidict==6.0.4 # via yarl -mypy==1.7.1 +mypy==1.8.0 # via pytest-mypy mypy-extensions==1.0.0 # via mypy packaging==23.2 # via - # -c requirements.txt + # -c requirements/main.txt # build # pyproject-api # pytest # sphinx # tox pip-tools==7.3.0 - # via -r dev-requirements.in + # via -r requirements/dev.in platformdirs==4.1.0 # via # tox @@ -123,16 +119,16 @@ pluggy==1.3.0 # tox pyasn1==0.5.1 # via - # -c requirements.txt + # -c requirements/main.txt # pyasn1-modules # rsa pyasn1-modules==0.3.0 # via - # -c requirements.txt + # -c requirements/main.txt # google-auth pycodestyle==2.11.1 # via flake8 -pyflakes==3.1.0 +pyflakes==3.2.0 # via flake8 pygments==2.17.2 # via @@ -142,106 +138,98 @@ pyproject-api==1.6.1 # via tox pyproject-hooks==1.0.0 # via build -pytest==7.4.3 +pytest==7.4.4 # via - # -r dev-requirements.in + # -r requirements/dev.in # pytest-cov # pytest-env # pytest-isort # pytest-mypy # pytest-vcr pytest-cov==4.1.0 - # via -r dev-requirements.in + # via -r requirements/dev.in pytest-env==1.1.3 - # via -r dev-requirements.in + # via -r requirements/dev.in pytest-isort==3.1.0 - # via -r dev-requirements.in + # via -r requirements/dev.in pytest-mypy==0.10.3 - # via -r dev-requirements.in + # via -r requirements/dev.in pytest-vcr==1.0.2 - # via -r dev-requirements.in + # via -r requirements/dev.in pytz==2023.3.post1 - # via -r dev-requirements.in + # via -r requirements/dev.in pyyaml==6.0.1 # via - # -c requirements.txt + # -c requirements/main.txt # vcrpy recommonmark==0.7.1 - # via -r dev-requirements.in + # via -r requirements/dev.in requests==2.31.0 # via - # -c requirements.txt + # -c requirements/main.txt # sphinx rsa==4.9 # via - # -c requirements.txt + # -c requirements/main.txt # google-auth snowballstemmer==2.2.0 # via sphinx soupsieve==2.5 # via beautifulsoup4 -sphinx==4.3.2 +sphinx==7.2.6 # via - # -r dev-requirements.in + # -r requirements/dev.in # furo # recommonmark # sphinx-autodoc-typehints # sphinx-basic-ng -sphinx-autodoc-typehints[type-comments]==1.17.1 - # via - # -r dev-requirements.in - # sphinx-autodoc-typehints + # sphinxcontrib-applehelp + # sphinxcontrib-devhelp + # sphinxcontrib-htmlhelp + # sphinxcontrib-qthelp + # sphinxcontrib-serializinghtml +sphinx-autodoc-typehints==1.25.2 + # via -r requirements/dev.in sphinx-basic-ng==1.0.0b2 # via furo -sphinxcontrib-applehelp==1.0.4 +sphinxcontrib-applehelp==1.0.7 # via sphinx -sphinxcontrib-devhelp==1.0.2 +sphinxcontrib-devhelp==1.0.5 # via sphinx -sphinxcontrib-htmlhelp==2.0.1 +sphinxcontrib-htmlhelp==2.0.4 # via sphinx sphinxcontrib-jsmath==1.0.1 # via sphinx -sphinxcontrib-qthelp==1.0.3 +sphinxcontrib-qthelp==1.0.6 # via sphinx -sphinxcontrib-serializinghtml==1.1.5 +sphinxcontrib-serializinghtml==1.1.9 # via sphinx -tomli==2.0.1 - # via - # build - # coverage - # mypy - # pip-tools - # pyproject-api - # pyproject-hooks - # pytest - # pytest-env - # tox tox==4.11.4 - # via -r dev-requirements.in + # via -r requirements/dev.in types-awscrt==0.20.0 # via botocore-stubs types-cryptography==3.3.23.2 # via types-jwt types-jwt==0.1.3 - # via -r dev-requirements.in -types-python-dateutil==2.8.19.14 - # via -r dev-requirements.in + # via -r requirements/dev.in +types-python-dateutil==2.8.19.20240106 + # via -r requirements/dev.in types-pytz==2023.3.1.1 - # via -r dev-requirements.in + # via -r requirements/dev.in types-pyyaml==6.0.12.12 - # via -r dev-requirements.in -types-requests==2.31.0.10 + # via -r requirements/dev.in +types-requests==2.31.0.20240106 # via google-auth-stubs -types-s3transfer==0.9.0 +types-s3transfer==0.10.0 # via boto3-stubs typing-extensions==4.9.0 # via - # -c requirements.txt + # -c requirements/main.txt # boto3-stubs # mypy urllib3==2.0.7 # via - # -c requirements.txt + # -c requirements/main.txt # requests # types-requests vcrpy==5.1.0 diff --git a/requirements.in b/requirements/main.in similarity index 100% rename from requirements.in rename to requirements/main.in diff --git a/requirements.txt b/requirements/main.txt similarity index 71% rename from requirements.txt rename to requirements/main.txt index 6bc2f41..f6e12e5 100644 --- a/requirements.txt +++ b/requirements/main.txt @@ -1,20 +1,20 @@ # -# This file is autogenerated by pip-compile with Python 3.10 +# This file is autogenerated by pip-compile with Python 3.11 # by the following command: # -# pip-compile --no-emit-index-url --output-file=requirements.txt requirements.in +# pip-compile --no-emit-index-url --output-file=requirements/main.txt requirements/main.in # anyio==4.2.0 # via azure-core azure-core==1.29.6 # via azure-storage-blob azure-storage-blob==12.19.0 - # via -r requirements.in + # via -r requirements/main.in blinker==1.7.0 # via flask -boto3==1.34.5 - # via -r requirements.in -botocore==1.34.5 +boto3==1.34.16 + # via -r requirements/main.in +botocore==1.34.16 # via # boto3 # s3transfer @@ -30,26 +30,24 @@ click==8.1.7 # via flask cryptography==41.0.7 # via - # -r requirements.in + # -r requirements/main.in # azure-storage-blob -exceptiongroup==1.2.0 - # via anyio figcan==0.0.4 - # via -r requirements.in + # via -r requirements/main.in flask==2.3.3 # via - # -r requirements.in + # -r requirements/main.in # flask-classful # flask-marshmallow flask-classful==0.16.0 - # via -r requirements.in + # via -r requirements/main.in flask-marshmallow==0.15.0 - # via -r requirements.in + # via -r requirements/main.in google-api-core==2.15.0 # via # google-cloud-core # google-cloud-storage -google-auth==2.25.2 +google-auth==2.26.1 # via # google-api-core # google-cloud-core @@ -57,7 +55,7 @@ google-auth==2.25.2 google-cloud-core==2.4.1 # via google-cloud-storage google-cloud-storage==2.14.0 - # via -r requirements.in + # via -r requirements/main.in google-crc32c==1.5.0 # via # google-cloud-storage @@ -70,13 +68,13 @@ idna==3.6 # via # anyio # requests -importlib-metadata==7.0.0 ; python_version < "3.13" - # via -r requirements.in +importlib-metadata==7.0.1 ; python_version < "3.13" + # via -r requirements/main.in isodate==0.6.1 # via azure-storage-blob itsdangerous==2.1.2 # via flask -jinja2==3.1.2 +jinja2==3.1.3 # via flask jmespath==1.0.1 # via @@ -86,19 +84,19 @@ markupsafe==2.1.3 # via # jinja2 # werkzeug -marshmallow==3.20.1 +marshmallow==3.20.2 # via # flask-marshmallow # marshmallow-enum # webargs marshmallow-enum==1.5.1 - # via -r requirements.in + # via -r requirements/main.in packaging==23.2 # via # flask-marshmallow # marshmallow # webargs -protobuf==4.25.1 +protobuf==4.25.2 # via # google-api-core # googleapis-common-protos @@ -111,15 +109,15 @@ pyasn1-modules==0.3.0 pycparser==2.21 # via cffi pyjwt==2.8.0 - # via -r requirements.in + # via -r requirements/main.in python-dateutil==2.8.2 # via - # -r requirements.in + # -r requirements/main.in # botocore python-dotenv==1.0.0 - # via -r requirements.in + # via -r requirements/main.in pyyaml==6.0.1 - # via -r requirements.in + # via -r requirements/main.in requests==2.31.0 # via # azure-core @@ -127,7 +125,7 @@ requests==2.31.0 # google-cloud-storage rsa==4.9 # via google-auth -s3transfer==0.9.0 +s3transfer==0.10.0 # via boto3 six==1.16.0 # via @@ -138,19 +136,18 @@ sniffio==1.3.0 # via anyio typing-extensions==4.9.0 # via - # -r requirements.in - # anyio + # -r requirements/main.in # azure-core # azure-storage-blob urllib3==2.0.7 # via # botocore # requests -webargs==8.3.0 - # via -r requirements.in +webargs==8.4.0 + # via -r requirements/main.in werkzeug==3.0.1 # via - # -r requirements.in + # -r requirements/main.in # flask zipp==3.17.0 # via importlib-metadata diff --git a/setup.py b/setup.py deleted file mode 100644 index 91f2dee..0000000 --- a/setup.py +++ /dev/null @@ -1,25 +0,0 @@ -from setuptools import find_packages, setup - -setup( - name="giftless", - packages=find_packages(exclude="./tests"), - version=open("VERSION").read(), - description="A Git LFS Server implementation in Python with support for pluggable backends", - author="Shahar Evron", - author_email="shahar.evron@datopian.com", - long_description=open("README.md").read(), - long_description_content_type="text/markdown", - install_requires=[ - "figcan", - "flask", - "flask-marshmallow", - "marshmallow-enum", - "pyyaml", - "PyJWT", - "webargs", - "python-dotenv", - "typing-extensions", - "flask-classful", - ], - include_package_data=True, -) diff --git a/tests/auth/test_auth.py b/tests/auth/test_auth.py index 5842090..2f8501e 100644 --- a/tests/auth/test_auth.py +++ b/tests/auth/test_auth.py @@ -1,12 +1,14 @@ """Unit tests for auth module """ +from typing import Any + import pytest from giftless.auth import allow_anon from giftless.auth.identity import DefaultIdentity, Permission -def test_default_identity_properties(): +def test_default_identity_properties() -> None: """Test the basic properties of the default identity object""" user = DefaultIdentity( "arthur", "kingofthebritons", "arthur@camelot.gov.uk" @@ -50,7 +52,7 @@ def test_default_identity_properties(): ), ], ) -def test_default_identity_denied_by_default(requested): +def test_default_identity_denied_by_default(requested: dict[str, Any]) -> None: user = DefaultIdentity( "arthur", "kingofthebritons", "arthur@camelot.gov.uk" ) @@ -103,7 +105,9 @@ def test_default_identity_denied_by_default(requested): ), ], ) -def test_default_identity_allow_specific_repo(requested, expected): +def test_default_identity_allow_specific_repo( + requested: dict[str, Any], expected: bool +) -> None: user = DefaultIdentity( "arthur", "kingofthebritons", "arthur@camelot.gov.uk" ) @@ -166,7 +170,9 @@ def test_default_identity_allow_specific_repo(requested, expected): ), ], ) -def test_default_identity_allow_specific_org_permissions(requested, expected): +def test_default_identity_allow_specific_org_permissions( + requested: dict[str, Any], expected: bool +) -> None: user = DefaultIdentity( "arthur", "kingofthebritons", "arthur@camelot.gov.uk" ) @@ -222,7 +228,9 @@ def test_default_identity_allow_specific_org_permissions(requested, expected): ), ], ) -def test_allow_anon_read_only(requested, expected): +def test_allow_anon_read_only( + requested: dict[str, Any], expected: bool +) -> None: """Test that an anon user with read only permissions works as expected""" user = allow_anon.read_only(None) assert expected is user.is_authorized(**requested) @@ -273,13 +281,15 @@ def test_allow_anon_read_only(requested, expected): ), ], ) -def test_allow_anon_read_write(requested, expected): +def test_allow_anon_read_write( + requested: dict[str, Any], expected: bool +) -> None: """Test that an anon user with read only permissions works as expected""" user = allow_anon.read_write(None) assert expected is user.is_authorized(**requested) -def test_anon_user_interface(): +def test_anon_user_interface() -> None: """Test that an anonymous user has the right interface""" user = allow_anon.read_only(None) assert isinstance(user, allow_anon.AnonymousUser) diff --git a/tests/auth/test_jwt.py b/tests/auth/test_jwt.py index 0e5cce3..773f1c7 100644 --- a/tests/auth/test_jwt.py +++ b/tests/auth/test_jwt.py @@ -1,6 +1,7 @@ import base64 import os from datetime import datetime, timedelta +from typing import Any import flask import jwt @@ -23,7 +24,7 @@ ) -def test_jwt_can_authorize_request_symmetric_key(app): +def test_jwt_can_authorize_request_symmetric_key(app: flask.Flask) -> None: """Test basic JWT authorizer functionality""" authz = JWTAuthenticator(private_key=JWT_HS_KEY, algorithm="HS256") token = _get_test_token() @@ -33,10 +34,11 @@ def test_jwt_can_authorize_request_symmetric_key(app): headers={"Authorization": f"Bearer {token}"}, ): identity = authz(flask.request) + assert identity is not None assert identity.id == "some-user-id" -def test_jwt_can_authorize_request_asymmetric_key(app): +def test_jwt_can_authorize_request_asymmetric_key(app: flask.Flask) -> None: """Test basic JWT authorizer functionality""" authz = factory(public_key_file=JWT_RS_PUB_KEY, algorithm="RS256") token = _get_test_token(algo="RS256") @@ -46,10 +48,11 @@ def test_jwt_can_authorize_request_asymmetric_key(app): headers={"Authorization": f"Bearer {token}"}, ): identity = authz(flask.request) + assert identity is not None assert identity.id == "some-user-id" -def test_jwt_can_authorize_request_token_in_qs(app): +def test_jwt_can_authorize_request_token_in_qs(app: flask.Flask) -> None: """Test basic JWT authorizer functionality""" authz = JWTAuthenticator(private_key=JWT_HS_KEY, algorithm="HS256") token = _get_test_token() @@ -57,10 +60,13 @@ def test_jwt_can_authorize_request_token_in_qs(app): f"/myorg/myrepo/objects/batch?jwt={token}", method="POST" ): identity = authz(flask.request) + assert identity is not None assert identity.id == "some-user-id" -def test_jwt_can_authorize_request_token_as_basic_password(app): +def test_jwt_can_authorize_request_token_as_basic_password( + app: flask.Flask, +) -> None: """Test that we can pass a JWT token as 'Basic' authorization password""" authz = JWTAuthenticator(private_key=JWT_HS_KEY, algorithm="HS256") token = _get_test_token() @@ -74,10 +80,13 @@ def test_jwt_can_authorize_request_token_as_basic_password(app): headers={"Authorization": f"Basic {auth_value}"}, ): identity = authz(flask.request) + assert identity is not None assert identity.id == "some-user-id" -def test_jwt_can_authorize_request_token_basic_password_disabled(app): +def test_jwt_can_authorize_request_token_basic_password_disabled( + app: flask.Flask, +) -> None: """Test that we can pass a JWT token as 'Basic' authorization password""" authz = JWTAuthenticator( private_key=JWT_HS_KEY, algorithm="HS256", basic_auth_user=None @@ -93,10 +102,10 @@ def test_jwt_can_authorize_request_token_basic_password_disabled(app): headers={"Authorization": f"Basic {auth_value}"}, ): identity = authz(flask.request) - assert None is identity + assert identity is None -def test_jwt_with_wrong_kid_doesnt_authorize_request(app): +def test_jwt_with_wrong_kid_doesnt_authorize_request(app: flask.Flask) -> None: """JWT authorizer only considers a JWT token if it has the right key ID in the header""" authz = JWTAuthenticator( private_key=JWT_HS_KEY, algorithm="HS256", key_id="must-be-this-key" @@ -108,10 +117,10 @@ def test_jwt_with_wrong_kid_doesnt_authorize_request(app): headers={"Authorization": f"Bearer {token}"}, ): identity = authz(flask.request) - assert None is identity + assert identity is None -def test_jwt_expired_throws_401(app): +def test_jwt_expired_throws_401(app: flask.Flask) -> None: """If we get a JWT token who's expired, we should raise a 401 error""" authz = JWTAuthenticator(private_key=JWT_HS_KEY, algorithm="HS256") token = _get_test_token(lifetime=-600) # expired 10 minutes ago @@ -124,7 +133,7 @@ def test_jwt_expired_throws_401(app): authz(flask.request) -def test_jwt_pre_authorize_action(): +def test_jwt_pre_authorize_action() -> None: authz = JWTAuthenticator( private_key=JWT_HS_KEY, algorithm="HS256", default_lifetime=120 ) @@ -152,7 +161,7 @@ def test_jwt_pre_authorize_action(): ) -def test_jwt_pre_authorize_action_custom_lifetime(): +def test_jwt_pre_authorize_action_custom_lifetime() -> None: authz = JWTAuthenticator( private_key=JWT_HS_KEY, algorithm="HS256", default_lifetime=120 ) @@ -395,7 +404,9 @@ def test_jwt_pre_authorize_action_custom_lifetime(): ), ], ) -def test_jwt_scopes_authorize_actions(app, scopes, auth_check, expected): +def test_jwt_scopes_authorize_actions( + app: flask.Flask, scopes: str, auth_check: dict[str, Any], expected: bool +) -> None: """Test that JWT token scopes can control authorization""" authz = JWTAuthenticator(private_key=JWT_HS_KEY, algorithm="HS256") token = _get_test_token(scopes=scopes) @@ -406,10 +417,11 @@ def test_jwt_scopes_authorize_actions(app, scopes, auth_check, expected): ): identity = authz(flask.request) + assert identity is not None assert identity.is_authorized(**auth_check) is expected -def test_jwt_scopes_authorize_actions_with_anon_user(app): +def test_jwt_scopes_authorize_actions_with_anon_user(app: flask.Flask) -> None: """Test that authorization works even if we don't have any user ID / email / name""" scopes = ["obj:myorg/myrepo/*"] authz = JWTAuthenticator(private_key=JWT_HS_KEY, algorithm="HS256") @@ -421,6 +433,7 @@ def test_jwt_scopes_authorize_actions_with_anon_user(app): ): identity = authz(flask.request) + assert identity is not None assert identity.is_authorized( organization="myorg", repo="myrepo", permission=Permission.READ ) @@ -521,7 +534,7 @@ def test_jwt_scopes_authorize_actions_with_anon_user(app): ), ], ) -def test_scope_parsing(scope_str, expected): +def test_scope_parsing(scope_str: str, expected: dict[str, Any]) -> None: """Test scope string parsing works as expected""" scope = Scope.from_string(scope_str) for k, v in expected.items(): @@ -543,12 +556,17 @@ def test_scope_parsing(scope_str, expected): ), ], ) -def test_scope_stringify(scope, expected): +def test_scope_stringify(scope: Scope, expected: str) -> None: """Test scope stringification works as expected""" assert expected == str(scope) -def _get_test_token(lifetime=300, headers=None, algo="HS256", **kwargs): +def _get_test_token( + lifetime: int = 300, + headers: dict[str, Any] | None = None, + algo: str = "HS256", + **kwargs: Any, +) -> str: payload = { "exp": datetime.now(tz=pytz.utc) + timedelta(seconds=lifetime), "sub": "some-user-id", @@ -559,9 +577,15 @@ def _get_test_token(lifetime=300, headers=None, algo="HS256", **kwargs): if algo == "HS256": key = JWT_HS_KEY elif algo == "RS256": - with open(JWT_RS_PRI_KEY) as f: + with open(JWT_RS_PRI_KEY, "rb") as f: key = f.read() else: raise ValueError(f"Don't know how to test algo: {algo}") - return jwt.encode(payload, key, algorithm=algo, headers=headers) + token = jwt.encode(payload, key, algorithm=algo, headers=headers) + # Type of jwt.encode() went from bytes to str in jwt 2.x, but the + # typing hints somehow aren't keeping up. This lets us do the + # right thing with jwt 2.x. + if isinstance(token, str): # type: ignore[unreachable] + return token # type: ignore[unreachable] + return token.decode("ascii") diff --git a/tests/conftest.py b/tests/conftest.py index 0d12fdb..7792037 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,14 +1,18 @@ +import pathlib import shutil +from typing import Generator, cast +import flask import pytest from flask.ctx import AppContext +from flask.testing import FlaskClient from giftless.app import init_app from giftless.auth import allow_anon, authentication @pytest.fixture -def storage_path(tmp_path): +def storage_path(tmp_path: pathlib.Path) -> Generator: path = tmp_path / "lfs-tests" path.mkdir() try: @@ -18,7 +22,7 @@ def storage_path(tmp_path): @pytest.fixture -def app(storage_path): +def app(storage_path: str) -> flask.Flask: """Session fixture to configure the Flask app""" app = init_app( additional_config={ @@ -35,7 +39,7 @@ def app(storage_path): @pytest.fixture -def app_context(app): +def app_context(app: flask.Flask) -> Generator: ctx = app.app_context() try: ctx.push() @@ -45,20 +49,24 @@ def app_context(app): @pytest.fixture -def test_client(app_context: AppContext): - test_client = app_context.app.test_client() +def test_client(app_context: AppContext) -> FlaskClient: + test_client: FlaskClient = app_context.app.test_client() return test_client @pytest.fixture def authz_full_access( - app_context, + app_context: AppContext, +) -> ( + Generator ): # needed to ensure we call init_authenticators before app context is destroyed """Fixture that enables full anonymous access to all actions for tests that use it """ try: - authentication.push_authenticator(allow_anon.read_write) + authentication.push_authenticator( + allow_anon.read_write # type:ignore[arg-type] + ) yield finally: authentication.init_authenticators(reload=True) diff --git a/tests/helpers.py b/tests/helpers.py index 1613247..80adc4a 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -1,9 +1,12 @@ """Test helpers """ import os +from typing import Any -def batch_request_payload(delete_keys=(), **kwargs): +def batch_request_payload( + delete_keys: list[str] = [], **kwargs: Any +) -> dict[str, Any]: """Generate sample batch request payload""" payload = { "operation": "download", @@ -19,7 +22,9 @@ def batch_request_payload(delete_keys=(), **kwargs): return payload -def create_file_in_storage(storage_path, org, repo, filename, size=1): +def create_file_in_storage( + storage_path: str, org: str, repo: str, filename: str, size: int = 1 +) -> None: """Put a dummy file in the storage path for a specific org / repo / oid combination This is useful where we want to test download / verify actions without relying on diff --git a/tests/storage/__init__.py b/tests/storage/__init__.py index 77cfca4..2c1f8bf 100644 --- a/tests/storage/__init__.py +++ b/tests/storage/__init__.py @@ -1,4 +1,5 @@ import io +from typing import Any, cast import pytest @@ -9,6 +10,9 @@ "2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824" ) +# The layering is bad in giftless.storage, leading to some strange choices +# for storage classes here. That should be refactored sometime. + class _CommonStorageAbstractTests: """Common tests for all storage backend types and interfaces @@ -16,7 +20,7 @@ class _CommonStorageAbstractTests: This should not be used directly, because it is inherited by other AbstractTest test suites. """ - def test_get_size(self, storage_backend): + def test_get_size(self, storage_backend: StreamingStorage) -> None: """Test getting the size of a stored object""" content = b"The contents of a file-like object" storage_backend.put("org/repo", ARBITRARY_OID, io.BytesIO(content)) @@ -24,18 +28,22 @@ def test_get_size(self, storage_backend): "org/repo", ARBITRARY_OID ) - def test_get_size_not_existing(self, storage_backend): + def test_get_size_not_existing( + self, storage_backend: StreamingStorage + ) -> None: """Test getting the size of a non-existing object raises an exception""" with pytest.raises(ObjectNotFound): storage_backend.get_size("org/repo", ARBITRARY_OID) - def test_exists_exists(self, storage_backend: StreamingStorage): + def test_exists_exists(self, storage_backend: StreamingStorage) -> None: """Test that calling exists on an existing object returns True""" content = b"The contents of a file-like object" storage_backend.put("org/repo", ARBITRARY_OID, io.BytesIO(content)) assert storage_backend.exists("org/repo", ARBITRARY_OID) - def test_exists_not_exists(self, storage_backend: StreamingStorage): + def test_exists_not_exists( + self, storage_backend: StreamingStorage + ) -> None: """Test that calling exists on a non-existing object returns False""" assert not storage_backend.exists("org/repo", ARBITRARY_OID) @@ -46,21 +54,27 @@ class _VerifiableStorageAbstractTests: This should not be used directly, because it is inherited by other AbstractTest test suites. """ - def test_verify_object_ok(self, storage_backend): + def test_verify_object_ok(self, storage_backend: StreamingStorage) -> None: content = b"The contents of a file-like object" + # put is part of StreamingStorage, not VerifiableStorage...but + # StreamingStorage implements VerifiableStorage storage_backend.put("org/repo", ARBITRARY_OID, io.BytesIO(content)) assert storage_backend.verify_object( "org/repo", ARBITRARY_OID, len(content) ) - def test_verify_object_wrong_size(self, storage_backend): + def test_verify_object_wrong_size( + self, storage_backend: StreamingStorage + ) -> None: content = b"The contents of a file-like object" storage_backend.put("org/repo", ARBITRARY_OID, io.BytesIO(content)) assert not storage_backend.verify_object( "org/repo", ARBITRARY_OID, len(content) + 2 ) - def test_verify_object_not_there(self, storage_backend): + def test_verify_object_not_there( + self, storage_backend: StreamingStorage + ) -> None: assert not storage_backend.verify_object("org/repo", ARBITRARY_OID, 0) @@ -73,7 +87,7 @@ class StreamingStorageAbstractTests( ``storage_backend`` that returns an appropriate storage backend object. """ - def test_put_get_object(self, storage_backend: StreamingStorage): + def test_put_get_object(self, storage_backend: StreamingStorage) -> None: """Test a full put-then-get cycle""" content = b"The contents of a file-like object" written = storage_backend.put( @@ -86,7 +100,9 @@ def test_put_get_object(self, storage_backend: StreamingStorage): fetched_content = b"".join(fetched) assert content == fetched_content - def test_get_raises_if_not_found(self, storage_backend: StreamingStorage): + def test_get_raises_if_not_found( + self, storage_backend: StreamingStorage + ) -> None: """Test that calling get for a non-existing object raises""" with pytest.raises(ObjectNotFound): storage_backend.get("org/repo", ARBITRARY_OID) @@ -101,20 +117,24 @@ class ExternalStorageAbstractTests( ``storage_backend`` that returns an appropriate storage backend object. """ - def test_get_upload_action(self, storage_backend: ExternalStorage): + def test_get_upload_action( + self, storage_backend: ExternalStorage + ) -> dict[str, Any]: action_spec = storage_backend.get_upload_action( "org/repo", ARBITRARY_OID, 100, 3600 ) - upload = action_spec["actions"]["upload"] + upload = cast(dict[str, Any], action_spec["actions"]["upload"]) assert upload["href"][0:4] == "http" assert upload["expires_in"] == 3600 return upload - def test_get_download_action(self, storage_backend): + def test_get_download_action( + self, storage_backend: ExternalStorage + ) -> dict[str, Any]: action_spec = storage_backend.get_download_action( "org/repo", ARBITRARY_OID, 100, 7200 ) - download = action_spec["actions"]["download"] + download = cast(dict[str, Any], action_spec["actions"]["download"]) assert download["href"][0:4] == "http" assert download["expires_in"] == 7200 return download diff --git a/tests/storage/test_amazon_s3.py b/tests/storage/test_amazon_s3.py index d9ead9c..db71749 100644 --- a/tests/storage/test_amazon_s3.py +++ b/tests/storage/test_amazon_s3.py @@ -4,6 +4,7 @@ from base64 import b64decode from binascii import unhexlify from collections.abc import Generator +from typing import Any import pytest @@ -50,7 +51,7 @@ def storage_backend() -> Generator[AmazonS3Storage, None, None]: @pytest.fixture(scope="module") -def vcr_config(): +def vcr_config() -> dict[str, Any]: live_tests = bool( os.environ.get("AWS_ACCESS_KEY_ID") and os.environ.get("AWS_SECRET_ACCESS_KEY") @@ -72,10 +73,16 @@ def vcr_config(): class TestAmazonS3StorageBackend( StreamingStorageAbstractTests, ExternalStorageAbstractTests ): - def test_get_upload_action(self, storage_backend: ExternalStorage): + # This is gross. Because we're extending the classes, it has to return + # the same thing, and upload uses the test method and returns a value + # which is not how tests usually work, and ugh. + def test_get_upload_action( + self, storage_backend: ExternalStorage + ) -> dict[str, Any]: upload = super().test_get_upload_action(storage_backend) assert upload["header"]["Content-Type"] == "application/octet-stream" b64_oid = upload["header"]["x-amz-checksum-sha256"] assert b64decode(b64_oid) == unhexlify(ARBITRARY_OID) + return upload # yuck diff --git a/tests/storage/test_azure.py b/tests/storage/test_azure.py index 6b91813..fcca9a8 100644 --- a/tests/storage/test_azure.py +++ b/tests/storage/test_azure.py @@ -2,10 +2,11 @@ """ import os from collections.abc import Generator +from typing import Any import pytest -from azure.core.exceptions import AzureError # type: ignore -from azure.storage.blob import BlobServiceClient # type: ignore +from azure.core.exceptions import AzureError +from azure.storage.blob import BlobServiceClient from giftless.storage.azure import AzureBlobsStorage @@ -55,7 +56,7 @@ def storage_backend() -> Generator[AzureBlobsStorage, None, None]: @pytest.fixture(scope="module") -def vcr_config(): +def vcr_config() -> dict[str, Any]: live_tests = bool( os.environ.get("AZURE_CONNECTION_STRING") and os.environ.get("AZURE_CONTAINER") diff --git a/tests/storage/test_google_cloud.py b/tests/storage/test_google_cloud.py index 01f745c..d7d8ea9 100644 --- a/tests/storage/test_google_cloud.py +++ b/tests/storage/test_google_cloud.py @@ -2,9 +2,10 @@ """ import os from collections.abc import Generator +from typing import Any import pytest -from google.api_core.exceptions import GoogleAPIError # type: ignore +from google.api_core.exceptions import GoogleAPIError from giftless.storage.google_cloud import GoogleCloudStorage @@ -100,7 +101,7 @@ def storage_backend() -> Generator[GoogleCloudStorage, None, None]: @pytest.fixture(scope="module") -def vcr_config(): +def vcr_config() -> dict[str, Any]: live_tests = bool( os.environ.get("GCP_ACCOUNT_KEY_FILE") and os.environ.get("GCP_PROJECT_NAME") diff --git a/tests/storage/test_local.py b/tests/storage/test_local.py index 02cb4a6..697230e 100644 --- a/tests/storage/test_local.py +++ b/tests/storage/test_local.py @@ -13,7 +13,7 @@ @pytest.fixture -def storage_dir(tmp_path) -> Generator[pathlib.Path, None, None]: +def storage_dir(tmp_path: pathlib.Path) -> Generator[pathlib.Path, None, None]: """Create a unique temp dir for testing storage""" dir = None try: @@ -26,13 +26,15 @@ def storage_dir(tmp_path) -> Generator[pathlib.Path, None, None]: @pytest.fixture -def storage_backend(storage_dir) -> LocalStorage: +def storage_backend(storage_dir: str) -> LocalStorage: """Provide a local storage backend for all local tests""" return LocalStorage(path=storage_dir) class TestLocalStorageBackend(StreamingStorageAbstractTests): - def test_local_path_created_on_init(self, storage_dir: pathlib.Path): + def test_local_path_created_on_init( + self, storage_dir: pathlib.Path + ) -> None: """Test that the local storage path is created on module init""" storage_path = str(storage_dir / "here") assert not os.path.exists(storage_path) diff --git a/tests/test_batch_api.py b/tests/test_batch_api.py index 8350cca..cd8ef9a 100644 --- a/tests/test_batch_api.py +++ b/tests/test_batch_api.py @@ -1,12 +1,15 @@ """Tests for schema definitions """ +from typing import cast + import pytest +from flask.testing import FlaskClient from .helpers import batch_request_payload, create_file_in_storage @pytest.mark.usefixtures("authz_full_access") -def test_upload_batch_request(test_client): +def test_upload_batch_request(test_client: FlaskClient) -> None: """Test basic batch API with a basic successful upload request""" request_payload = batch_request_payload(operation="upload") response = test_client.post( @@ -16,7 +19,7 @@ def test_upload_batch_request(test_client): assert response.status_code == 200 assert response.content_type == "application/vnd.git-lfs+json" - payload = response.json + payload = cast(dict, response.json) assert "message" not in payload assert payload["transfer"] == "basic" assert len(payload["objects"]) == 1 @@ -29,7 +32,9 @@ def test_upload_batch_request(test_client): assert "verify" in object["actions"] -def test_download_batch_request(test_client, storage_path): +def test_download_batch_request( + test_client: FlaskClient, storage_path: str +) -> None: """Test basic batch API with a basic successful upload request""" request_payload = batch_request_payload(operation="download") oid = request_payload["objects"][0]["oid"] @@ -42,7 +47,7 @@ def test_download_batch_request(test_client, storage_path): assert response.status_code == 200 assert response.content_type == "application/vnd.git-lfs+json" - payload = response.json + payload = cast(dict, response.json) assert "message" not in payload assert payload["transfer"] == "basic" assert len(payload["objects"]) == 1 @@ -55,8 +60,8 @@ def test_download_batch_request(test_client, storage_path): def test_download_batch_request_two_files_one_missing( - test_client, storage_path -): + test_client: FlaskClient, storage_path: str +) -> None: """Test batch API with a two object download request where one file 404""" request_payload = batch_request_payload(operation="download") oid = request_payload["objects"][0]["oid"] @@ -72,7 +77,7 @@ def test_download_batch_request_two_files_one_missing( assert response.status_code == 200 assert response.content_type == "application/vnd.git-lfs+json" - payload = response.json + payload = cast(dict, response.json) assert "message" not in payload assert payload["transfer"] == "basic" assert len(payload["objects"]) == 2 @@ -90,7 +95,9 @@ def test_download_batch_request_two_files_one_missing( assert object["error"]["code"] == 404 -def test_download_batch_request_two_files_missing(test_client): +def test_download_batch_request_two_files_missing( + test_client: FlaskClient, +) -> None: """Test batch API with a two object download request where one file 404""" request_payload = batch_request_payload(operation="download") request_payload["objects"].append({"oid": "12345679", "size": 5555}) @@ -102,15 +109,15 @@ def test_download_batch_request_two_files_missing(test_client): assert response.status_code == 404 assert response.content_type == "application/vnd.git-lfs+json" - payload = response.json + payload = cast(dict, response.json) assert "message" in payload assert "objects" not in payload assert "transfer" not in payload def test_download_batch_request_two_files_one_mismatch( - test_client, storage_path -): + test_client: FlaskClient, storage_path: str +) -> None: """Test batch API with a two object download request where one file 422""" request_payload = batch_request_payload(operation="download") request_payload["objects"].append({"oid": "12345679", "size": 8}) @@ -137,7 +144,7 @@ def test_download_batch_request_two_files_one_mismatch( assert response.status_code == 200 assert response.content_type == "application/vnd.git-lfs+json" - payload = response.json + payload = cast(dict, response.json) assert "message" not in payload assert payload["transfer"] == "basic" assert len(payload["objects"]) == 2 @@ -155,7 +162,9 @@ def test_download_batch_request_two_files_one_mismatch( assert object["error"]["code"] == 422 -def test_download_batch_request_one_file_mismatch(test_client, storage_path): +def test_download_batch_request_one_file_mismatch( + test_client: FlaskClient, storage_path: str +) -> None: """Test batch API with a two object download request where one file 422""" request_payload = batch_request_payload(operation="download") create_file_in_storage( @@ -173,15 +182,15 @@ def test_download_batch_request_one_file_mismatch(test_client, storage_path): assert response.status_code == 422 assert response.content_type == "application/vnd.git-lfs+json" - payload = response.json + payload = cast(dict, response.json) assert "message" in payload assert "objects" not in payload assert "transfer" not in payload def test_download_batch_request_two_files_different_errors( - test_client, storage_path -): + test_client: FlaskClient, storage_path: str +) -> None: """Test batch API with a two object download request where one file is missing and one is mismatch""" request_payload = batch_request_payload(operation="download") request_payload["objects"].append({"oid": "12345679", "size": 8}) @@ -200,7 +209,7 @@ def test_download_batch_request_two_files_different_errors( assert response.status_code == 422 assert response.content_type == "application/vnd.git-lfs+json" - payload = response.json + payload = cast(dict, response.json) assert "message" in payload assert "objects" not in payload assert "transfer" not in payload diff --git a/tests/test_error_responses.py b/tests/test_error_responses.py index 767d96f..ade7495 100644 --- a/tests/test_error_responses.py +++ b/tests/test_error_responses.py @@ -1,9 +1,11 @@ """Tests for schema definitions """ +from flask.testing import FlaskClient + from .helpers import batch_request_payload -def test_error_response_422(test_client): +def test_error_response_422(test_client: FlaskClient) -> None: """Test an invalid payload error""" response = test_client.post( "/myorg/myrepo/objects/batch", @@ -12,19 +14,19 @@ def test_error_response_422(test_client): assert response.status_code == 422 assert response.content_type == "application/vnd.git-lfs+json" - assert "message" in response.json + assert "message" in response.json # type:ignore[operator] -def test_error_response_404(test_client): +def test_error_response_404(test_client: FlaskClient) -> None: """Test a bad route error""" response = test_client.get("/now/for/something/completely/different") assert response.status_code == 404 assert response.content_type == "application/vnd.git-lfs+json" - assert "message" in response.json + assert "message" in response.json # type:ignore[operator] -def test_error_response_403(test_client): +def test_error_response_403(test_client: FlaskClient) -> None: """Test that we get Forbidden when trying to upload with the default read-only setup""" response = test_client.post( "/myorg/myrepo/objects/batch", @@ -33,4 +35,4 @@ def test_error_response_403(test_client): assert response.status_code == 403 assert response.content_type == "application/vnd.git-lfs+json" - assert "message" in response.json + assert "message" in response.json # type:ignore[operator] diff --git a/tests/test_middleware.py b/tests/test_middleware.py index 1a1303e..bc23aae 100644 --- a/tests/test_middleware.py +++ b/tests/test_middleware.py @@ -1,6 +1,10 @@ """Tests for using middleware and some specific middleware """ +from typing import Any, cast + import pytest +from flask import Flask +from flask.testing import FlaskClient from giftless.app import init_app @@ -8,7 +12,7 @@ @pytest.fixture -def app(storage_path): +def app(storage_path: str) -> Flask: """Session fixture to configure the Flask app""" app = init_app( additional_config={ @@ -34,7 +38,9 @@ def app(storage_path): @pytest.mark.usefixtures("authz_full_access") -def test_upload_request_with_x_forwarded_middleware(test_client): +def test_upload_request_with_x_forwarded_middleware( + test_client: FlaskClient, +) -> None: """Test the ProxyFix middleware generates correct URLs if X-Forwarded headers are set""" request_payload = batch_request_payload(operation="upload") response = test_client.post( @@ -42,7 +48,9 @@ def test_upload_request_with_x_forwarded_middleware(test_client): ) assert response.status_code == 200 - href = response.json["objects"][0]["actions"]["upload"]["href"] + json = cast(dict[str, Any], response.json) + upload_action = json["objects"][0]["actions"]["upload"] + href = upload_action["href"] assert href == "http://localhost/myorg/myrepo/objects/storage/12345678" response = test_client.post( @@ -57,7 +65,9 @@ def test_upload_request_with_x_forwarded_middleware(test_client): ) assert response.status_code == 200 - href = response.json["objects"][0]["actions"]["upload"]["href"] + json = cast(dict[str, Any], response.json) + upload_action = json["objects"][0]["actions"]["upload"] + href = upload_action["href"] assert ( href == "https://mycompany.xyz:1234/lfs/myorg/myrepo/objects/storage/12345678" diff --git a/tests/test_schema.py b/tests/test_schema.py index f9d9ce9..6929626 100644 --- a/tests/test_schema.py +++ b/tests/test_schema.py @@ -17,7 +17,7 @@ (batch_request_payload(delete_keys=["ref", "transfers"])), ], ) -def test_batch_request_schema_valid(input): +def test_batch_request_schema_valid(input: str) -> None: parsed = schema.BatchRequest().load(input) assert parsed @@ -36,18 +36,18 @@ def test_batch_request_schema_valid(input): (batch_request_payload(objects=[{"oid": "123abc", "size": -12}])), ], ) -def test_batch_request_schema_invalid(input): +def test_batch_request_schema_invalid(input: str) -> None: with pytest.raises(ValidationError): schema.BatchRequest().load(input) -def test_batch_request_default_transfer(): +def test_batch_request_default_transfer() -> None: input = batch_request_payload(delete_keys=["transfers"]) parsed = schema.BatchRequest().load(input) assert ["basic"] == parsed["transfers"] -def test_object_schema_accepts_x_fields(): +def test_object_schema_accepts_x_fields() -> None: payload = { "oid": "123abc", "size": 1212, @@ -62,7 +62,7 @@ def test_object_schema_accepts_x_fields(): assert parsed["extra"]["disposition"] == "inline" -def test_object_schema_rejects_unknown_fields(): +def test_object_schema_rejects_unknown_fields() -> None: payload = { "oid": "123abc", "size": 1212, diff --git a/tests/transfer/conftest.py b/tests/transfer/conftest.py index 5523247..2c49f83 100644 --- a/tests/transfer/conftest.py +++ b/tests/transfer/conftest.py @@ -1,12 +1,14 @@ """Some global fixtures for transfer tests """ +from typing import Generator + import pytest from giftless import transfer @pytest.fixture -def reset_registered_transfers(): +def reset_registered_transfers() -> Generator: """Reset global registered transfer adapters for each module""" adapters = dict(transfer._registered_adapters) try: diff --git a/tests/transfer/test_basic_external_adapter.py b/tests/transfer/test_basic_external_adapter.py index 3119d8d..4d18a2a 100644 --- a/tests/transfer/test_basic_external_adapter.py +++ b/tests/transfer/test_basic_external_adapter.py @@ -3,11 +3,12 @@ import pytest +from giftless.storage import ExternalStorage from giftless.storage.exc import ObjectNotFound from giftless.transfer import basic_external -def test_factory_returns_object(): +def test_factory_returns_object() -> None: """Test that the basic_external factory returns the right object(s)""" base_url = "https://s4.example.com/" lifetime = 300 @@ -24,7 +25,7 @@ def test_factory_returns_object(): @pytest.mark.usefixtures("app_context") -def test_upload_action_new_file(): +def test_upload_action_new_file() -> None: adapter = basic_external.factory( f"{__name__}:MockExternalStorageBackend", {}, @@ -52,7 +53,7 @@ def test_upload_action_new_file(): @pytest.mark.usefixtures("app_context") -def test_upload_action_extras_are_passed(): +def test_upload_action_extras_are_passed() -> None: adapter = basic_external.factory( f"{__name__}:MockExternalStorageBackend", {}, 900 ) @@ -80,7 +81,7 @@ def test_upload_action_extras_are_passed(): @pytest.mark.usefixtures("app_context") -def test_upload_action_existing_file(): +def test_upload_action_existing_file() -> None: storage = MockExternalStorageBackend() adapter = basic_external.BasicExternalBackendTransferAdapter(storage, 900) @@ -97,7 +98,7 @@ def test_upload_action_existing_file(): @pytest.mark.usefixtures("app_context") -def test_download_action_existing_file(): +def test_download_action_existing_file() -> None: storage = MockExternalStorageBackend() adapter = basic_external.BasicExternalBackendTransferAdapter(storage, 900) @@ -120,7 +121,7 @@ def test_download_action_existing_file(): @pytest.mark.usefixtures("app_context") -def test_download_action_non_existing_file(): +def test_download_action_non_existing_file() -> None: storage = MockExternalStorageBackend() adapter = basic_external.BasicExternalBackendTransferAdapter(storage, 900) @@ -136,7 +137,7 @@ def test_download_action_non_existing_file(): @pytest.mark.usefixtures("app_context") -def test_download_action_size_mismatch(): +def test_download_action_size_mismatch() -> None: storage = MockExternalStorageBackend() adapter = basic_external.BasicExternalBackendTransferAdapter(storage, 900) @@ -152,7 +153,7 @@ def test_download_action_size_mismatch(): @pytest.mark.usefixtures("app_context") -def test_download_action_extras_are_passed(): +def test_download_action_extras_are_passed() -> None: storage = MockExternalStorageBackend() adapter = basic_external.BasicExternalBackendTransferAdapter(storage, 900) @@ -176,7 +177,7 @@ def test_download_action_extras_are_passed(): } -class MockExternalStorageBackend(basic_external.ExternalStorage): +class MockExternalStorageBackend(ExternalStorage): """A mock adapter for the basic external transfer adapter Typically, "external" backends are cloud providers - so this backend can @@ -184,7 +185,9 @@ class MockExternalStorageBackend(basic_external.ExternalStorage): accessing an actual cloud provider. """ - def __init__(self, base_url: str = "https://cloudstorage.example.com/"): + def __init__( + self, base_url: str = "https://cloudstorage.example.com/" + ) -> None: self.existing_objects: dict[tuple[str, str], int] = {} self.base_url = base_url @@ -243,7 +246,7 @@ def _get_signed_url( oid: str, expires_in: int, extra: Optional[dict[str, Any]] = None, - ): + ) -> str: url = f"{self.base_url}{prefix}/{oid}?expires_in={expires_in}" if extra: url = f"{url}&{urlencode(extra, doseq=False)}" diff --git a/tests/transfer/test_module.py b/tests/transfer/test_module.py index f04d3c3..7f424fb 100644 --- a/tests/transfer/test_module.py +++ b/tests/transfer/test_module.py @@ -1,5 +1,7 @@ """Test common transfer module functionality """ +from typing import Any + import pytest from giftless import transfer @@ -15,7 +17,9 @@ ], ) @pytest.mark.usefixtures("reset_registered_transfers") -def test_transfer_adapter_matching(register, requested, expected): +def test_transfer_adapter_matching( + register: list[str], requested: list[str], expected: str +) -> None: for adapter in register: transfer.register_adapter(adapter, transfer.TransferAdapter()) actual = transfer.match_transfer_adapter(requested) @@ -23,7 +27,7 @@ def test_transfer_adapter_matching(register, requested, expected): assert isinstance(actual[1], transfer.TransferAdapter) -def test_transfer_adapter_matching_nomatch(): +def test_transfer_adapter_matching_nomatch() -> None: for adapter in ["foobar", "basic", "bizbaz"]: transfer.register_adapter(adapter, transfer.TransferAdapter()) with pytest.raises(ValueError): diff --git a/tox.ini b/tox.ini index 8f2bbd3..938f80b 100644 --- a/tox.ini +++ b/tox.ini @@ -9,8 +9,8 @@ isolated_build=true [testenv] deps = - -rrequirements.txt - -rdev-requirements.txt + -rrequirements/main.txt + -rrequirements/dev.txt [testenv:coverage-report] description = Compile coverage from each test run.