From 6ed320632f910761aa38b7bbcb408aab37047a2e Mon Sep 17 00:00:00 2001 From: Federico Stagni Date: Wed, 3 Dec 2025 14:51:38 +0100 Subject: [PATCH 1/6] fix: pre-commit: use hukkin/mdformat instead of executablebooks/mkdformat --- .pre-commit-config.yaml | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 458cee037..9a2eef311 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -30,7 +30,7 @@ repos: - id: check-added-large-files - repo: https://github.com/astral-sh/ruff-pre-commit - rev: 'v0.14.7' + rev: "v0.14.7" hooks: - id: ruff args: ["--fix", "--show-fixes"] @@ -51,28 +51,28 @@ repos: - boto3-stubs[essential] exclude: ^(diracx-client/src/diracx/client/_generated|diracx-[a-z]+/tests/|diracx-testing/|build|extensions/gubbins/gubbins-client/src/gubbins/client/_generated) - - repo: https://github.com/executablebooks/mdformat + - repo: https://github.com/hukkin/mdformat rev: 1.0.0 hooks: - - id: mdformat - args: ["--number"] - additional_dependencies: - - mdformat-mkdocs - - mdformat-gfm - - mdformat-black + - id: mdformat + args: ["--number"] + additional_dependencies: + - mdformat-mkdocs + - mdformat-gfm + - mdformat-black - repo: https://github.com/codespell-project/codespell rev: v2.4.1 hooks: - - id: codespell - args: ["-w"] + - id: codespell + args: ["-w"] - repo: local hooks: - id: check-lollygag name: check for lollygag types: [python] - entry: '(?i)lollygag' + entry: "(?i)lollygag" language: pygrep files: ^diracx @@ -83,4 +83,4 @@ repos: entry: pixi run -e default python .github/workflows/generate_pixi_tasks_doc.py language: system pass_filenames: false - files: ^pixi\.toml$|^pixi\.lock$ # only run if pixi files change + files: ^pixi\.toml$|^pixi\.lock$ # only run if pixi files change From a1e0f5ae31acfc28bd5b6d934650eb9addcd39c0 Mon Sep 17 00:00:00 2001 From: Federico Stagni Date: Wed, 3 Dec 2025 17:37:53 +0100 Subject: [PATCH 2/6] fix: pre-commit: added blacken-docs --- .pre-commit-config.yaml | 8 +++++- diracx-db/src/diracx/db/os/utils.py | 3 +++ diracx-db/src/diracx/db/sql/utils/base.py | 1 + docs/dev/explanations/components/api.md | 3 ++- docs/dev/explanations/components/cli.md | 5 ++-- docs/dev/explanations/components/routes.md | 31 ++++++++++++++++------ docs/dev/reference/coding-conventions.md | 6 +++-- docs/dev/reference/dependency-injection.md | 2 +- 8 files changed, 44 insertions(+), 15 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 9a2eef311..6f640f5b5 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -59,7 +59,6 @@ repos: additional_dependencies: - mdformat-mkdocs - mdformat-gfm - - mdformat-black - repo: https://github.com/codespell-project/codespell rev: v2.4.1 @@ -67,6 +66,13 @@ repos: - id: codespell args: ["-w"] + - repo: https://github.com/adamchainz/blacken-docs + rev: 1.20.0 + hooks: + - id: blacken-docs + additional_dependencies: + - black==22.12.0 + - repo: local hooks: - id: check-lollygag diff --git a/diracx-db/src/diracx/db/os/utils.py b/diracx-db/src/diracx/db/os/utils.py index ea5d292e6..d46331035 100644 --- a/diracx-db/src/diracx/db/os/utils.py +++ b/diracx-db/src/diracx/db/os/utils.py @@ -66,9 +66,12 @@ class BaseOSDB(metaclass=ABCMeta): MyDBClass = BaseOSDB.available_implementations(db_name)[0] db = MyDBClass(conn_params) + await db.init() + async with db.client_context: async with db: # Do something with the OpenSearch client + pass ``` """ diff --git a/diracx-db/src/diracx/db/sql/utils/base.py b/diracx-db/src/diracx/db/sql/utils/base.py index efb30d642..88415631b 100644 --- a/diracx-db/src/diracx/db/sql/utils/base.py +++ b/diracx-db/src/diracx/db/sql/utils/base.py @@ -84,6 +84,7 @@ class BaseSQLDB(metaclass=ABCMeta): async with db: # Do something in the first transaction # Commit will be called automatically + pass async with db: # This transaction will be rolled back due to the exception diff --git a/docs/dev/explanations/components/api.md b/docs/dev/explanations/components/api.md index a55222e3c..aba9c8e5d 100644 --- a/docs/dev/explanations/components/api.md +++ b/docs/dev/explanations/components/api.md @@ -18,7 +18,8 @@ from .utils import with_client @with_client -async def create_sandbox(paths: list[Path], *, client: AsyncDiracClient) -> str: ... +async def create_sandbox(paths: list[Path], *, client: AsyncDiracClient) -> str: + ... ``` In this example, `paths` are the parameters of the API. The `@with_client` decorator allows the method to be called without manually managing the client: diff --git a/docs/dev/explanations/components/cli.md b/docs/dev/explanations/components/cli.md index 94e3835c5..7f6e5ad3e 100644 --- a/docs/dev/explanations/components/cli.md +++ b/docs/dev/explanations/components/cli.md @@ -49,10 +49,11 @@ TODO: WRONG - To associate the command with `dirac`, import the module in `src/diracx/__init__.py`: ```python - from . import command + from . import my_command + ... - app.add_typer(.app, name="") + app.add_typer(my_command.app, name="my_command") ``` Users can then call the CLI: diff --git a/docs/dev/explanations/components/routes.md b/docs/dev/explanations/components/routes.md index 833d160a0..b0bb3be78 100644 --- a/docs/dev/explanations/components/routes.md +++ b/docs/dev/explanations/components/routes.md @@ -28,6 +28,8 @@ Example route definition: ```python @router.post("/search", responses=EXAMPLE_RESPONSES) +def search(): + pass ``` ## Dependency Injection @@ -66,7 +68,8 @@ Usage example: ```python @router.get("/openid-configuration") -async def get_openid_configuration(settings: AuthSettings): ... +async def get_openid_configuration(settings: AuthSettings): + ... ``` ### User Info @@ -77,7 +80,12 @@ To retrieve information about the current user, depend on `AuthorizedUserInfo`. @router.get("/userinfo") async def userinfo( user_info: Annotated[AuthorizedUserInfo, Depends(verify_dirac_access_token)], -): ... +): + ... +``` + +``` +pass ``` **TODO:** Consider avoiding the need to manually specify the annotation. @@ -88,7 +96,8 @@ To extract information from the central DIRAC configuration: ```python @router.post("/summary") -async def summary(config: Annotated[Config, Depends(ConfigSource.create)]): ... +async def summary(config: Annotated[Config, Depends(ConfigSource.create)]): + ... ``` The `Config` object is cached efficiently between requests and automatically refreshed. It is strongly typed and immutable for the duration of a request. @@ -106,7 +115,8 @@ from diracx.routers.dependencies import JobDB, JobLoggingDB @router.delete("/{job_id}") -async def delete_single_job(job_db: JobDB, job_logging_db: JobLoggingDB): ... +async def delete_single_job(job_db: JobDB, job_logging_db: JobLoggingDB): + ... ``` There are advanced and uncommon scenarios where committing a transaction is necessary even when returning an error response (e.g., revoking tokens in the database and returning an error to a potentially malicious user). In such cases, explicitly committing the transaction before raising an exception is crucial. Without this explicit commit, the intended changes would be rolled back along with the transaction, leading to unintended consequences: @@ -114,9 +124,9 @@ There are advanced and uncommon scenarios where committing a transaction is nece ```python from diracx.routers.dependencies import AuthDB + @router.post("/token") -async def token(auth_db: AuthDB, ...) - ... +async def token(auth_db: AuthDB): if refresh_token_attributes["status"] == RefreshTokenStatus.REVOKED: # Revoke all the user tokens associated with the subject await auth_db.revoke_user_refresh_tokens(sub) @@ -129,6 +139,8 @@ async def token(auth_db: AuthDB, ...) raise HTTPException(status_code=401) ``` +```` + Refer to the [SQLAlchemy documentation](https://docs.sqlalchemy.org/en/20/core/pooling.html) for more details. ### OpenSearch Databases @@ -142,7 +154,8 @@ from diracx.routers.dependencies import JobParametersDB @router.post("/search", responses=EXAMPLE_RESPONSES) -async def search(job_parameters_db: JobParametersDB): ... +async def search(job_parameters_db: JobParametersDB): + ... ``` ## Permission Management @@ -183,7 +196,8 @@ from .access_policies import open_access @open_access @router.get("/") -async def serve_config(): ... +async def serve_config(): + ... ``` Implementing a new `AccessPolicy` is done by: @@ -206,3 +220,4 @@ To ensure consistency, the following rules must be followed: - All routers must be tagged and the first tag becomes the name of the sub-client. - The name of the route becomes the name of the client method. - Uses of `fastapi.Form` must specify a `description`. +```` diff --git a/docs/dev/reference/coding-conventions.md b/docs/dev/reference/coding-conventions.md index dcfdedf76..aa5194380 100644 --- a/docs/dev/reference/coding-conventions.md +++ b/docs/dev/reference/coding-conventions.md @@ -32,7 +32,8 @@ import pytest @pytest.fixture -def my_ficture(): ... +def my_ficture(): + ... ``` @@ -44,7 +45,8 @@ from pytest import fixture @fixture -def my_ficture(): ... +def my_ficture(): + ... ``` diff --git a/docs/dev/reference/dependency-injection.md b/docs/dev/reference/dependency-injection.md index e92935253..b3e79ec01 100644 --- a/docs/dev/reference/dependency-injection.md +++ b/docs/dev/reference/dependency-injection.md @@ -81,7 +81,7 @@ For advanced scenarios requiring explicit transaction commits (e.g., revoking to ```python @router.post("/token") -async def token(auth_db: AuthDB, ...): +async def token(auth_db: AuthDB): if refresh_token_attributes["status"] == RefreshTokenStatus.REVOKED: # Revoke all the user tokens associated with the subject await auth_db.revoke_user_refresh_tokens(sub) From a9a7150c4203d6deff20427b3b73152c72e82844 Mon Sep 17 00:00:00 2001 From: Federico Stagni Date: Wed, 3 Dec 2025 17:45:02 +0100 Subject: [PATCH 3/6] chore: imports (from mypy) --- diracx-client/src/diracx/client/patches/jobs/common.py | 4 ++-- diracx-core/src/diracx/core/s3.py | 3 ++- diracx-logic/src/diracx/logic/auth/utils.py | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/diracx-client/src/diracx/client/patches/jobs/common.py b/diracx-client/src/diracx/client/patches/jobs/common.py index 597cda308..afff22927 100644 --- a/diracx-client/src/diracx/client/patches/jobs/common.py +++ b/diracx-client/src/diracx/client/patches/jobs/common.py @@ -14,8 +14,8 @@ import json from io import BytesIO, IOBase -from typing import Any, IO, Dict, TypedDict, Union, Unpack, cast, Literal - +from typing import Any, IO, Dict, Union, Unpack, cast, Literal +from typing_extensions import TypedDict from diracx.core.models import SearchSpec diff --git a/diracx-core/src/diracx/core/s3.py b/diracx-core/src/diracx/core/s3.py index 4a2d1229d..b8874752b 100644 --- a/diracx-core/src/diracx/core/s3.py +++ b/diracx-core/src/diracx/core/s3.py @@ -11,7 +11,8 @@ import asyncio import base64 -from typing import TYPE_CHECKING, TypedDict, cast +from typing import TYPE_CHECKING, cast +from typing_extensions import TypedDict from botocore.errorfactory import ClientError diff --git a/diracx-logic/src/diracx/logic/auth/utils.py b/diracx-logic/src/diracx/logic/auth/utils.py index 184fc9f6a..549ae3c7c 100644 --- a/diracx-logic/src/diracx/logic/auth/utils.py +++ b/diracx-logic/src/diracx/logic/auth/utils.py @@ -4,6 +4,7 @@ import hashlib import json import secrets +from typing_extensions import TypedDict import httpx from cachetools import TTLCache @@ -11,7 +12,6 @@ from joserfc import jwt from joserfc.jwk import KeySet from joserfc.jwt import Claims, JWTClaimsRegistry -from typing_extensions import TypedDict from uuid_utils import UUID from diracx.core.config.schema import Config From a8cb165b5c4ca1b86e98d8c79c3fbebb6d4c6d0e Mon Sep 17 00:00:00 2001 From: Federico Stagni Date: Wed, 3 Dec 2025 17:50:17 +0100 Subject: [PATCH 4/6] fix: fixed mypy python version to 3.11 --- pyproject.toml | 163 +++++++++++++++++++++++-------------------------- 1 file changed, 78 insertions(+), 85 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 34f03db8c..8c0d23b96 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -6,18 +6,13 @@ requires-python = ">=3.11" keywords = [] license = { text = "GPL-3.0-only" } classifiers = [ - "Intended Audience :: Science/Research", - "License :: OSI Approved :: GNU General Public License v3 (GPLv3)", - "Programming Language :: Python :: 3", - "Topic :: Scientific/Engineering", - "Topic :: System :: Distributed Computing", -] -dependencies = [ - "diracx-api", - "diracx-cli", - "diracx-client", - "diracx-core", + "Intended Audience :: Science/Research", + "License :: OSI Approved :: GNU General Public License v3 (GPLv3)", + "Programming Language :: Python :: 3", + "Topic :: Scientific/Engineering", + "Topic :: System :: Distributed Computing", ] +dependencies = ["diracx-api", "diracx-cli", "diracx-client", "diracx-core"] dynamic = ["version"] [project.optional-dependencies] @@ -35,43 +30,41 @@ bypass-selection = true [tool.ruff] src = ["diracx-*/src", "diracx-*/tests"] -exclude = [ - "diracx-client/src/diracx/client", -] +exclude = ["diracx-client/src/diracx/client"] [tool.ruff.lint] select = [ - "E", # pycodestyle errors - "F", # pyflakes - "B", # flake8-bugbear - "I", # isort - "PLE", # pylint errors - "D", # pydocstyle - # "UP", # pyUpgrade - "FLY", # flynt - "DTZ", # flake8-datetimez - "S", # flake8-bandit - "N", # pep8-naming + "E", # pycodestyle errors + "F", # pyflakes + "B", # flake8-bugbear + "I", # isort + "PLE", # pylint errors + "D", # pydocstyle + # "UP", # pyUpgrade + "FLY", # flynt + "DTZ", # flake8-datetimez + "S", # flake8-bandit + "N", # pep8-naming ] ignore = [ - "B905", - "B008", - "B006", - "S101", # bandit: use of assert https://docs.astral.sh/ruff/rules/assert/ - "D203", - "D213", - # TODO: Maybe enable these - "D100", - "D101", - "D102", - "D103", - "D104", - "D105", - "D107", - # TODO: These should be re-enabled after fixing - "D205", - "D401", - "D404", + "B905", + "B008", + "B006", + "S101", # bandit: use of assert https://docs.astral.sh/ruff/rules/assert/ + "D203", + "D213", + # TODO: Maybe enable these + "D100", + "D101", + "D102", + "D103", + "D104", + "D105", + "D107", + # TODO: These should be re-enabled after fixing + "D205", + "D401", + "D404", ] [tool.ruff.lint.isort] @@ -88,11 +81,11 @@ required-imports = ["from __future__ import annotations"] [tool.ruff.lint.flake8-bugbear] # Allow default arguments like, e.g., `data: List[str] = fastapi.Query(None)`. extend-immutable-calls = [ - "fastapi.Depends", - "fastapi.Query", - "fastapi.Path", - "fastapi.Body", - "fastapi.Header", + "fastapi.Depends", + "fastapi.Query", + "fastapi.Path", + "fastapi.Body", + "fastapi.Header", ] [tool.ruff.lint.pycodestyle] @@ -108,36 +101,34 @@ profile = "black" [tool.codespell] skip = [ - "diracx-client/src/diracx/client/_generated/*", - "diracx-[a-z]*/tests/*", - "diracx-testing/*", - "extensions/gubbins/gubbins-client/src/gubbins/client/_generated/*", - "extensions/gubbins/gubbins-*/tests/*", -] -ignore-words-list = [ - "CheckIn", - "dependant", + "diracx-client/src/diracx/client/_generated/*", + "diracx-[a-z]*/tests/*", + "diracx-testing/*", + "extensions/gubbins/gubbins-client/src/gubbins/client/_generated/*", + "extensions/gubbins/gubbins-*/tests/*", ] +ignore-words-list = ["CheckIn", "dependant"] [tool.mypy] +python_version = "3.11" files = [ - "diracx-api/src/**/*.py", - "diracx-cli/src/**/*.py", - "diracx-client/src/**/_patch.py", - "diracx-core/src/**/*.py", - "diracx-db/src/**/*.py", - "diracx-logic/src/**/*.py", - "diracx-routers/src/**/*.py", + "diracx-api/src/**/*.py", + "diracx-cli/src/**/*.py", + "diracx-client/src/**/_patch.py", + "diracx-core/src/**/*.py", + "diracx-db/src/**/*.py", + "diracx-logic/src/**/*.py", + "diracx-routers/src/**/*.py", ] mypy_path = [ - "$MYPY_CONFIG_FILE_DIR/diracx-api/src", - "$MYPY_CONFIG_FILE_DIR/diracx-cli/src", - "$MYPY_CONFIG_FILE_DIR/diracx-client/src", - "$MYPY_CONFIG_FILE_DIR/diracx-core/src", - "$MYPY_CONFIG_FILE_DIR/diracx-db/src", - "$MYPY_CONFIG_FILE_DIR/diracx-logic/src", - "$MYPY_CONFIG_FILE_DIR/diracx-routers/src", + "$MYPY_CONFIG_FILE_DIR/diracx-api/src", + "$MYPY_CONFIG_FILE_DIR/diracx-cli/src", + "$MYPY_CONFIG_FILE_DIR/diracx-client/src", + "$MYPY_CONFIG_FILE_DIR/diracx-core/src", + "$MYPY_CONFIG_FILE_DIR/diracx-db/src", + "$MYPY_CONFIG_FILE_DIR/diracx-logic/src", + "$MYPY_CONFIG_FILE_DIR/diracx-routers/src", ] plugins = ["sqlalchemy.ext.mypy.plugin", "pydantic.mypy"] allow_redefinition = true @@ -160,23 +151,25 @@ log_cli_level = "INFO" xfail_strict = true filterwarnings = ["default"] testpaths = [ - "diracx-api/tests", - "diracx-cli/tests", - "diracx-client/tests", - "diracx-core/tests", - "diracx-db/tests", - "diracx-routers/tests", + "diracx-api/tests", + "diracx-cli/tests", + "diracx-client/tests", + "diracx-core/tests", + "diracx-db/tests", + "diracx-routers/tests", ] addopts = [ - "-v", - "--cov=diracx", - "--cov-report=term-missing", - "-pdiracx.testing", - "-pdiracx.testing.osdb", - "--import-mode=importlib", - "-ra", "--strict-config", "--strict-markers", + "-v", + "--cov=diracx", + "--cov-report=term-missing", + "-pdiracx.testing", + "-pdiracx.testing.osdb", + "--import-mode=importlib", + "-ra", + "--strict-config", + "--strict-markers", ] asyncio_mode = "auto" markers = [ - "enabled_dependencies: List of dependencies which should be available to the FastAPI test client", + "enabled_dependencies: List of dependencies which should be available to the FastAPI test client", ] From 34c9733eaa49bf441ae796951f95ac6d5b7fadc5 Mon Sep 17 00:00:00 2001 From: Federico Stagni Date: Wed, 3 Dec 2025 17:52:59 +0100 Subject: [PATCH 5/6] fix: pre-commit: use ruff-check instead of ruff (legacy) --- .pre-commit-config.yaml | 2 +- diracx-core/src/diracx/core/s3.py | 2 +- diracx-logic/src/diracx/logic/auth/utils.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 6f640f5b5..601a6f070 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -32,7 +32,7 @@ repos: - repo: https://github.com/astral-sh/ruff-pre-commit rev: "v0.14.7" hooks: - - id: ruff + - id: ruff-check args: ["--fix", "--show-fixes"] - id: ruff-format diff --git a/diracx-core/src/diracx/core/s3.py b/diracx-core/src/diracx/core/s3.py index b8874752b..6eaf04581 100644 --- a/diracx-core/src/diracx/core/s3.py +++ b/diracx-core/src/diracx/core/s3.py @@ -12,9 +12,9 @@ import asyncio import base64 from typing import TYPE_CHECKING, cast -from typing_extensions import TypedDict from botocore.errorfactory import ClientError +from typing_extensions import TypedDict from .models import ChecksumAlgorithm diff --git a/diracx-logic/src/diracx/logic/auth/utils.py b/diracx-logic/src/diracx/logic/auth/utils.py index 549ae3c7c..184fc9f6a 100644 --- a/diracx-logic/src/diracx/logic/auth/utils.py +++ b/diracx-logic/src/diracx/logic/auth/utils.py @@ -4,7 +4,6 @@ import hashlib import json import secrets -from typing_extensions import TypedDict import httpx from cachetools import TTLCache @@ -12,6 +11,7 @@ from joserfc import jwt from joserfc.jwk import KeySet from joserfc.jwt import Claims, JWTClaimsRegistry +from typing_extensions import TypedDict from uuid_utils import UUID from diracx.core.config.schema import Config From 91b4c54355ea938acca6812fbdfec295b52cf88a Mon Sep 17 00:00:00 2001 From: Federico Stagni Date: Wed, 3 Dec 2025 17:55:32 +0100 Subject: [PATCH 6/6] fix: pre-commit: explicitely adding mypy strict=false --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 8c0d23b96..f486bb555 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -112,6 +112,7 @@ ignore-words-list = ["CheckIn", "dependant"] [tool.mypy] python_version = "3.11" +strict = false files = [ "diracx-api/src/**/*.py", "diracx-cli/src/**/*.py",