diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 458cee037..601a6f070 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -30,9 +30,9 @@ 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 + - id: ruff-check args: ["--fix", "--show-fixes"] - id: ruff-format @@ -51,28 +51,34 @@ 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 - repo: https://github.com/codespell-project/codespell rev: v2.4.1 hooks: - - id: codespell - args: ["-w"] + - 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 name: check for lollygag types: [python] - entry: '(?i)lollygag' + entry: "(?i)lollygag" language: pygrep files: ^diracx @@ -83,4 +89,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 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..6eaf04581 100644 --- a/diracx-core/src/diracx/core/s3.py +++ b/diracx-core/src/diracx/core/s3.py @@ -11,9 +11,10 @@ import asyncio import base64 -from typing import TYPE_CHECKING, TypedDict, cast +from typing import TYPE_CHECKING, cast from botocore.errorfactory import ClientError +from typing_extensions import TypedDict from .models import ChecksumAlgorithm 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) diff --git a/pyproject.toml b/pyproject.toml index 34f03db8c..f486bb555 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,35 @@ 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" +strict = false 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 +152,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", ]