From 10d30e8fea9ed8a0db9bf856c4e6116fd239ded4 Mon Sep 17 00:00:00 2001 From: martynia Date: Thu, 16 Jul 2026 09:38:26 +0200 Subject: [PATCH 1/2] docs: modify rest style comments: docconvert --input rest --output google --- diracx-core/src/diracx/core/sources.py | 12 ++------- diracx-db/src/diracx/db/sql/auth/db.py | 4 +-- diracx-db/src/diracx/db/sql/job/db.py | 22 ++++++++++------ diracx-logic/src/diracx/logic/auth/utils.py | 2 +- diracx-logic/src/diracx/logic/jobs/status.py | 3 ++- .../src/diracx/logic/task_queues/priority.py | 14 ++++++---- .../src/diracx/routers/access_policies.py | 10 ++++--- diracx-routers/src/diracx/routers/factory.py | 26 ++++++++----------- .../gubbins-cli/src/gubbins/cli/config.py | 8 ++---- .../gubbins-cli/src/gubbins/cli/lollygag.py | 15 +++-------- .../gubbins-db/src/gubbins/db/sql/jobs/db.py | 13 +++------- .../src/gubbins/db/sql/lollygag/db.py | 3 +-- .../gubbins/routers/lollygag/access_policy.py | 3 +-- .../src/gubbins/routers/lollygag/lollygag.py | 3 +-- .../src/gubbins/routers/well_known.py | 3 +-- pyproject.toml | 2 ++ 16 files changed, 63 insertions(+), 80 deletions(-) diff --git a/diracx-core/src/diracx/core/sources.py b/diracx-core/src/diracx/core/sources.py index 2521f0269..aaf5237a9 100644 --- a/diracx-core/src/diracx/core/sources.py +++ b/diracx-core/src/diracx/core/sources.py @@ -80,22 +80,14 @@ def read_raw(self, hexsha: str, modified: datetime) -> T: """ def read(self) -> T: - """Load the source from the backend with appropriate caching. - - :raises: diracx.core.exceptions.NotReadyError if the source is being loaded still - :raises: git.exc.BadName if version does not exist - """ + """Load the source from the backend with appropriate caching.""" hexsha = self._revision_cache.get( "latest_revision", self._read_work, blocking=True ) return self._content_cache[hexsha] async def read_non_blocking(self) -> T: - """Load the source from the backend with appropriate caching. - - :raises: diracx.core.exceptions.NotReadyError if the source is being loaded still - :raises: git.exc.BadName if version does not exist - """ + """Load the source from the backend with appropriate caching.""" hexsha = self._revision_cache.get( "latest_revision", self._read_work, blocking=False ) diff --git a/diracx-db/src/diracx/db/sql/auth/db.py b/diracx-db/src/diracx/db/sql/auth/db.py index a35a0ad19..808db9d29 100644 --- a/diracx-db/src/diracx/db/sql/auth/db.py +++ b/diracx-db/src/diracx/db/sql/auth/db.py @@ -159,7 +159,7 @@ async def device_flow_validate_user_code( Returns the scope field for the given user_code - :raises: + Raises: NoResultFound if no such user code currently Pending """ stmt = select(DeviceFlows.scope).where( @@ -273,7 +273,7 @@ async def authorization_flow_insert_id_token( ) -> tuple[str, str]: """Return code, redirect_uri. - :raises: AuthorizationError if no such uuid or status not pending. + Raises: AuthorizationError if no such uuid or status not pending. """ # Hash the code to avoid leaking information code = secrets.token_urlsafe() diff --git a/diracx-db/src/diracx/db/sql/job/db.py b/diracx-db/src/diracx/db/sql/job/db.py index 38d2cf011..28171e138 100644 --- a/diracx-db/src/diracx/db/sql/job/db.py +++ b/diracx-db/src/diracx/db/sql/job/db.py @@ -200,10 +200,12 @@ async def set_properties( All the jobs must update the same properties. - :param properties: {job_id : {prop1: val1, prop2:val2} - :param update_timestamp: if True, update the LastUpdate to now + Args: + properties: {job_id : {prop1: val1, prop2:val2} + update_timestamp: if True, update the LastUpdate to now - :return rowcount + Returns: + rowcount """ # Check that all we always update the same set of properties @@ -238,9 +240,10 @@ async def add_heartbeat_data( This is instead handled by the `diracx.logic.jobs.status.set_job_statuses` as it involves updating multiple databases. - :param job_id: the job id - :param dynamic_data: mapping of the dynamic data to store, - e.g. {"AvailableDiskSpace": 123} + Args: + job_id: the job id + dynamic_data: mapping of the dynamic data to store, e.g. + {"AvailableDiskSpace": 123} """ if extra_fields := set(dynamic_data) - self.heartbeat_fields: raise InvalidQueryError( @@ -261,8 +264,11 @@ async def add_heartbeat_data( async def get_job_commands(self, job_ids: Iterable[int]) -> list[JobCommand]: """Get a command to be passed to the job together with the next heartbeat. - :param job_ids: the job ids - :return: mapping of job id to list of commands + Args: + job_ids: the job ids + + Returns: + mapping of job id to list of commands """ # Get the commands stmt = ( diff --git a/diracx-logic/src/diracx/logic/auth/utils.py b/diracx-logic/src/diracx/logic/auth/utils.py index 6e0f44da0..549f36f87 100644 --- a/diracx-logic/src/diracx/logic/auth/utils.py +++ b/diracx-logic/src/diracx/logic/auth/utils.py @@ -243,7 +243,7 @@ def parse_and_validate_scope( * properties are known return dict with group and properties. - :raises: + Raises: * ValueError in case the scope isn't valid """ scopes = set(scope.split(" ")) diff --git a/diracx-logic/src/diracx/logic/jobs/status.py b/diracx-logic/src/diracx/logic/jobs/status.py index 7a1b507ff..ee69b782b 100644 --- a/diracx-logic/src/diracx/logic/jobs/status.py +++ b/diracx-logic/src/diracx/logic/jobs/status.py @@ -103,7 +103,8 @@ async def set_job_statuses( logging information in the JobLoggingDB. The status dict has datetime as a key and status information dictionary as values. - :raises: JobNotFound if the job is not found in one of the DBs + Raises: + JobNotFound if the job is not found in one of the DBs """ # check that the datetime contains timezone info for job_id, status in status_changes.items(): diff --git a/diracx-logic/src/diracx/logic/task_queues/priority.py b/diracx-logic/src/diracx/logic/task_queues/priority.py index 0e848778e..ed4475611 100644 --- a/diracx-logic/src/diracx/logic/task_queues/priority.py +++ b/diracx-logic/src/diracx/logic/task_queues/priority.py @@ -99,11 +99,15 @@ async def calculate_priority( ) -> dict[float, list[int]]: """Calculate the priority for each TQ given a share. - :param tq_dict: dict of {tq_id: prio} - :param all_tqs_data: dict of {tq_id: {tq_data}}, where tq_data is a dict of {field: value} - :param share: share to be distributed among TQs - :param allow_bg_tqs: allow background TQs to be used - :return: dict of {priority: [tq_ids]} + Args: + tq_dict: dict of {tq_id: prio} + all_tqs_data: dict of {tq_id: {tq_data}}, where tq_data is a + dict of {field: value} + share: share to be distributed among TQs + allow_bg_tqs: allow background TQs to be used + + Returns: + dict of {priority: [tq_ids]} """ def is_background(tq_priority: float, allow_bg_tqs: bool) -> bool: diff --git a/diracx-routers/src/diracx/routers/access_policies.py b/diracx-routers/src/diracx/routers/access_policies.py index c2e498d74..4ef056404 100644 --- a/diracx-routers/src/diracx/routers/access_policies.py +++ b/diracx-routers/src/diracx/routers/access_policies.py @@ -9,7 +9,6 @@ * have the open_access decorator to make explicit that it does not implement policy * have a callable and call it that will perform the access policy - Adding a new policy: 1. Create a class that inherits from BaseAccessPolicy and implement the ``policy`` and ``enrich_tokens`` methods 2. create an entry in diracx.access_policy entrypoints @@ -97,9 +96,12 @@ def enrich_tokens( Content can be whatever is desired inside the access or refresh payload. - :param access_payload: access token payload - :param refresh_payload: refresh token payload - :returns: extra content for both payload + Args: + access_payload: access token payload + refresh_payload: refresh token payload + + Returns: + extra content for both payload """ return {}, {} diff --git a/diracx-routers/src/diracx/routers/factory.py b/diracx-routers/src/diracx/routers/factory.py index bbd58e227..e1cd449b9 100644 --- a/diracx-routers/src/diracx/routers/factory.py +++ b/diracx-routers/src/diracx/routers/factory.py @@ -115,21 +115,17 @@ def create_app_inner( the actual behavior we are interested in for settings, DBs or policy. This allows an extension to override any of these components - - :param enabled_system: - this contains the name of all the routers we have to load - :param all_service_settings: - list of instance of each Settings type required - :param database_urls: - dict . When testing, sqlite urls are used - :param os_database_conn_kwargs: - containing all the parameters the OpenSearch client takes - :param config_source: - Source of the configuration to use - :param all_access_policies: - - - + Args: + enabled_systems: this contains the name of all the routers we + have to load + all_service_settings: list of instance of each Settings type + required + database_urls: dict . When testing, sqlite urls + are used + os_database_conn_kwargs: containing all the + parameters the OpenSearch client takes + config_source: Source of the configuration to use + all_access_policies: """ app = DiracFastAPI() diff --git a/extensions/gubbins/gubbins-cli/src/gubbins/cli/config.py b/extensions/gubbins/gubbins-cli/src/gubbins/cli/config.py index cb40af64a..37a5045f0 100644 --- a/extensions/gubbins/gubbins-cli/src/gubbins/cli/config.py +++ b/extensions/gubbins/gubbins-cli/src/gubbins/cli/config.py @@ -1,6 +1,4 @@ -""" -This just shows how to extend and modify an existing CLI -""" +"""This just shows how to extend and modify an existing CLI""" # In order to extend it, just import the app from DiracX from __future__ import annotations @@ -12,9 +10,7 @@ @app.async_command() async def gubbins_extra(): - """ - Add an extra command - """ + """Add an extra command""" print("Adding something extra") diff --git a/extensions/gubbins/gubbins-cli/src/gubbins/cli/lollygag.py b/extensions/gubbins/gubbins-cli/src/gubbins/cli/lollygag.py index 846de24be..0d58be1b7 100644 --- a/extensions/gubbins/gubbins-cli/src/gubbins/cli/lollygag.py +++ b/extensions/gubbins/gubbins-cli/src/gubbins/cli/lollygag.py @@ -1,6 +1,4 @@ -""" -This shows how to create a new subcommand -""" +"""This shows how to create a new subcommand""" from __future__ import annotations @@ -15,16 +13,13 @@ @app.command() def hello(): - """ - This is just to make sure that the CLI extension mechanism works - """ + """This is just to make sure that the CLI extension mechanism works""" print("Shagadelic, Baby!") @app.async_command() async def get_owners(): - """ - This makes a proper use of the AsyncGubbinsClient to call + """This makes a proper use of the AsyncGubbinsClient to call a method specific to Gubbins """ async with AsyncGubbinsClient() as api: @@ -34,9 +29,7 @@ async def get_owners(): @app.async_command() async def sensei(): - """ - This function is only here to test the GUBBINS_SENSEI property - """ + """This function is only here to test the GUBBINS_SENSEI property""" async with AsyncGubbinsClient() as api: secrets = await api.lollygag.get_gubbins_secrets() print(secrets) diff --git a/extensions/gubbins/gubbins-db/src/gubbins/db/sql/jobs/db.py b/extensions/gubbins/gubbins-db/src/gubbins/db/sql/jobs/db.py index bb15bcf94..55409095e 100644 --- a/extensions/gubbins/gubbins-db/src/gubbins/db/sql/jobs/db.py +++ b/extensions/gubbins/gubbins-db/src/gubbins/db/sql/jobs/db.py @@ -7,8 +7,7 @@ class GubbinsJobDB(JobDB): - """ - This DB extends the diracx JobDB. + """This DB extends the diracx JobDB. All methods from the parent DB are accessible """ @@ -16,17 +15,14 @@ class GubbinsJobDB(JobDB): metadata = JobDBBase.metadata async def insert_gubbins_info(self, job_id: int, info: str): - """ - This is a new method that makes use of a new table. - """ + """This is a new method that makes use of a new table.""" stmt = insert(GubbinsInfo).values(job_id=job_id, info=info) await self.conn.execute(stmt) async def get_job_jdls( # type: ignore[override] self, job_ids, original: bool = False, with_info=False ) -> dict: - """ - This method modifies the one in the parent class: + """This method modifies the one in the parent class: * adds an extra argument * changes the return type @@ -50,8 +46,7 @@ async def get_job_jdls( # type: ignore[override] return result async def set_job_attributes(self, job_data): - """ - This method modified the one in the parent class, + """This method modified the one in the parent class, without changing the argument nor the return type Also, this method is called by the router via the status_utility diff --git a/extensions/gubbins/gubbins-db/src/gubbins/db/sql/lollygag/db.py b/extensions/gubbins/gubbins-db/src/gubbins/db/sql/lollygag/db.py index 2964a5a38..fc08e8850 100644 --- a/extensions/gubbins/gubbins-db/src/gubbins/db/sql/lollygag/db.py +++ b/extensions/gubbins/gubbins-db/src/gubbins/db/sql/lollygag/db.py @@ -9,8 +9,7 @@ class LollygagDB(BaseSQLDB): - """ - This LollygagDB is just to illustrate some important aspect of writing + """This LollygagDB is just to illustrate some important aspect of writing DB classes in DiracX. It is mostly pure SQLAlchemy, with a few convention diff --git a/extensions/gubbins/gubbins-routers/src/gubbins/routers/lollygag/access_policy.py b/extensions/gubbins/gubbins-routers/src/gubbins/routers/lollygag/access_policy.py index 85813c5a7..267c7ca8e 100644 --- a/extensions/gubbins/gubbins-routers/src/gubbins/routers/lollygag/access_policy.py +++ b/extensions/gubbins/gubbins-routers/src/gubbins/routers/lollygag/access_policy.py @@ -1,5 +1,4 @@ -""" -Lollygag dummy AccessPolicy +"""Lollygag dummy AccessPolicy Makes sure we can use Gubbins specific property """ diff --git a/extensions/gubbins/gubbins-routers/src/gubbins/routers/lollygag/lollygag.py b/extensions/gubbins/gubbins-routers/src/gubbins/routers/lollygag/lollygag.py index 91abba791..fb3b13ba5 100644 --- a/extensions/gubbins/gubbins-routers/src/gubbins/routers/lollygag/lollygag.py +++ b/extensions/gubbins/gubbins-routers/src/gubbins/routers/lollygag/lollygag.py @@ -1,5 +1,4 @@ -""" -This router makes use of the new LollygagDB. +"""This router makes use of the new LollygagDB. It uses the the Lollygag AccessPolicy (which itself requires the Gubbins property) """ diff --git a/extensions/gubbins/gubbins-routers/src/gubbins/routers/well_known.py b/extensions/gubbins/gubbins-routers/src/gubbins/routers/well_known.py index 1e33de91d..ff8053edc 100644 --- a/extensions/gubbins/gubbins-routers/src/gubbins/routers/well_known.py +++ b/extensions/gubbins/gubbins-routers/src/gubbins/routers/well_known.py @@ -1,5 +1,4 @@ -""" -Illustrate how to extend/overwrite a diracx router. +"""Illustrate how to extend/overwrite a diracx router. It : * changes slightly the return type * uses the Gubbins specific configuration diff --git a/pyproject.toml b/pyproject.toml index ce5106bdf..99c0efb25 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -99,6 +99,8 @@ max-line-length = 120 [tool.ruff.format] docstring-code-format = true +[tool.ruff.lint.pydocstyle] +convention = "google" [tool.isort] profile = "black" From cdd4cdd095135134a48790b9a9b7921368527325 Mon Sep 17 00:00:00 2001 From: martynia Date: Tue, 21 Jul 2026 11:32:30 +0100 Subject: [PATCH 2/2] docs: implement suggestions from a review --- diracx-db/src/diracx/db/sql/auth/db.py | 7 ++++--- diracx-db/src/diracx/db/sql/job/db.py | 2 +- diracx-logic/src/diracx/logic/auth/utils.py | 2 +- diracx-logic/src/diracx/logic/jobs/status.py | 2 +- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/diracx-db/src/diracx/db/sql/auth/db.py b/diracx-db/src/diracx/db/sql/auth/db.py index 808db9d29..e56f7c1e9 100644 --- a/diracx-db/src/diracx/db/sql/auth/db.py +++ b/diracx-db/src/diracx/db/sql/auth/db.py @@ -160,7 +160,7 @@ async def device_flow_validate_user_code( Returns the scope field for the given user_code Raises: - NoResultFound if no such user code currently Pending + NoResultFound: if no such user code currently Pending """ stmt = select(DeviceFlows.scope).where( DeviceFlows.user_code == user_code, @@ -271,9 +271,10 @@ async def insert_authorization_flow( async def authorization_flow_insert_id_token( self, uuid: str, id_token: dict[str, str], max_validity: int ) -> tuple[str, str]: - """Return code, redirect_uri. + """Return the authorization code and redirect URI. - Raises: AuthorizationError if no such uuid or status not pending. + Raises: + AuthorizationError: If no such UUID exists or the flow is not pending. """ # Hash the code to avoid leaking information code = secrets.token_urlsafe() diff --git a/diracx-db/src/diracx/db/sql/job/db.py b/diracx-db/src/diracx/db/sql/job/db.py index 28171e138..94ba4df63 100644 --- a/diracx-db/src/diracx/db/sql/job/db.py +++ b/diracx-db/src/diracx/db/sql/job/db.py @@ -205,7 +205,7 @@ async def set_properties( update_timestamp: if True, update the LastUpdate to now Returns: - rowcount + rowcount """ # Check that all we always update the same set of properties diff --git a/diracx-logic/src/diracx/logic/auth/utils.py b/diracx-logic/src/diracx/logic/auth/utils.py index 549f36f87..d905db692 100644 --- a/diracx-logic/src/diracx/logic/auth/utils.py +++ b/diracx-logic/src/diracx/logic/auth/utils.py @@ -244,7 +244,7 @@ def parse_and_validate_scope( return dict with group and properties. Raises: - * ValueError in case the scope isn't valid + ValueError: in case the scope isn't valid """ scopes = set(scope.split(" ")) diff --git a/diracx-logic/src/diracx/logic/jobs/status.py b/diracx-logic/src/diracx/logic/jobs/status.py index ee69b782b..38d0339d3 100644 --- a/diracx-logic/src/diracx/logic/jobs/status.py +++ b/diracx-logic/src/diracx/logic/jobs/status.py @@ -104,7 +104,7 @@ async def set_job_statuses( as a key and status information dictionary as values. Raises: - JobNotFound if the job is not found in one of the DBs + JobNotFound: if the job is not found in one of the DBs """ # check that the datetime contains timezone info for job_id, status in status_changes.items():