Apply feedback and improve docstring consistency

.github/workflows/make_release.py

@@ -7,7 +7,7 @@
 
 
 def make_release(version, commit_hash, release_notes=""):
-    """Create a new GitHub release using the given data
+    """Create a new GitHub release using the given data.
 
     This function always makes a pre-release first to ensure the "latest" release never corresponds
     to one without artifacts uploaded. If the new version number is not a pre-release, as

diracx-cli/src/diracx/cli/internal/__init__.py

@@ -26,7 +26,7 @@
 
 @app.command()
 def generate_cs(config_repo: str):
-    """Generate a minimal DiracX configuration repository"""
+    """Generate a minimal DiracX configuration repository."""
     # TODO: The use of TypeAdapter should be moved in to typer itself
     config_repo = TypeAdapter(ConfigSourceUrl).validate_python(config_repo)
     if config_repo.scheme != "git+file" or config_repo.path is None:
@@ -58,8 +58,7 @@ def add_vo(
     idp_url: Annotated[str, typer.Option()],
     idp_client_id: Annotated[str, typer.Option()],
 ):
-    """Add a registry entry (vo) to an existing configuration repository"""
-
+    """Add a registry entry (vo) to an existing configuration repository."""
     # TODO: The use of TypeAdapter should be moved in to typer itself
     config_repo = TypeAdapter(ConfigSourceUrl).validate_python(config_repo)
     if config_repo.scheme != "git+file" or config_repo.path is None:
@@ -102,8 +101,7 @@ def add_group(
     group: Annotated[str, typer.Option()],
     properties: list[str] = ["NormalUser"],
 ):
-    """Add a group to an existing vo in the configuration repository"""
-
+    """Add a group to an existing vo in the configuration repository."""
     # TODO: The use of TypeAdapter should be moved in to typer itself
     config_repo = TypeAdapter(ConfigSourceUrl).validate_python(config_repo)
     if config_repo.scheme != "git+file" or config_repo.path is None:
@@ -139,8 +137,7 @@ def add_user(
     sub: Annotated[str, typer.Option()],
     preferred_username: Annotated[str, typer.Option()],
 ):
-    """Add a user to an existing vo and group"""
-
+    """Add a user to an existing vo and group."""
     # TODO: The use of TypeAdapter should be moved in to typer itself
     config_repo = TypeAdapter(ConfigSourceUrl).validate_python(config_repo)
     if config_repo.scheme != "git+file" or config_repo.path is None:
@@ -184,7 +181,7 @@ def add_user(
 
 
 def update_config_and_commit(repo_path: Path, config: Config, message: str):
-    """Update the yaml file in the repo and commit it"""
+    """Update the yaml file in the repo and commit it."""
     repo = git.Repo(repo_path)
     yaml_path = repo_path / "default.yml"
     typer.echo(f"Writing back configuration to {yaml_path}", err=True)

diracx-cli/src/diracx/cli/internal/legacy.py

@@ -53,7 +53,7 @@ class ConversionConfig(BaseModel):
 
 @app.command()
 def cs_sync(old_file: Path, new_file: Path):
-    """Load the old CS and convert it to the new YAML format"""
+    """Load the old CS and convert it to the new YAML format."""
     if not os.environ.get("DIRAC_COMPAT_ENABLE_CS_CONVERSION"):
         raise RuntimeError(
             "DIRAC_COMPAT_ENABLE_CS_CONVERSION must be set for the conversion to be possible"
@@ -86,8 +86,7 @@ def cs_sync(old_file: Path, new_file: Path):
 
 
 def _apply_fixes(raw):
-    """Modify raw in place to make any layout changes between the old and new structure"""
-
+    """Modify raw in place to make any layout changes between the old and new structure."""
     conv_config = ConversionConfig.model_validate(raw["DiracX"]["CsSync"])
 
     raw.pop("DiracX", None)
@@ -184,10 +183,10 @@ def generate_helm_values(
     ),
     output_file: Path = Option(help="Where to dump the yam file"),
 ):
-    """Generate an initial values.yaml to run a DiracX installation
-    compatible with a DIRAC instance. The file is not complete, and needs
-    manual editing"""
+    """Generate an initial values.yaml to run a DiracX installation.
 
+    The file generated is not complete, and needs manual editing.
+    """
     helm_values = {
         "developer": {"enabled": False},
         "init-cs": {"enabled": True},

diracx-cli/tests/test_jobs.py

@@ -41,7 +41,6 @@ async def jdl_file():
 
 async def test_submit(with_cli_login, jdl_file, capfd):
     """Test submitting a job using a JDL file."""
-
     with open(jdl_file, "r") as temp_file:
         await cli.jobs.submit([temp_file])
 
@@ -52,7 +51,6 @@ async def test_submit(with_cli_login, jdl_file, capfd):
 
 async def test_search(with_cli_login, jdl_file, capfd):
     """Test searching for jobs."""
-
     # Submit 20 jobs
     with open(jdl_file, "r") as temp_file:
         await cli.jobs.submit([temp_file] * 20)

diracx-client/tests/test_regenerate.py

@@ -20,7 +20,7 @@ def test_client(client_factory):
 
 
 def test_regenerate_client(test_client, tmp_path):
-    """Regenerate the AutoREST client and run pre-commit checks on it
+    """Regenerate the AutoREST client and run pre-commit checks on it.
 
     This test is skipped by default, and can be enabled by passing
     --regenerate-client to pytest. It is intended to be run manually

diracx-core/src/diracx/core/config/__init__.py

@@ -1,6 +1,6 @@
-"""
-This module implements the logic of the configuration server side.
-This is where all the backend abstraction and the caching logic takes place
+"""This module implements the logic of the configuration server side.
+
+This is where all the backend abstraction and the caching logic takes place.
 """
 
 from __future__ import annotations
@@ -46,11 +46,10 @@ def _apply_default_scheme(value: str) -> str:
 
 
 class ConfigSource(metaclass=ABCMeta):
-    """
-    This classe is the abstract base class that should be used everywhere
+    """This class is the abstract base class that should be used everywhere
     throughout the code.
     It acts as a factory for concrete implementations
-    See the abstractmethods to implement a concrete class
+    See the abstractmethods to implement a concrete class.
     """
 
     # Keep a mapping between the scheme and the class
@@ -64,24 +63,21 @@ def __init__(self, *, backend_url: ConfigSourceUrl) -> None: ...
     def latest_revision(self) -> tuple[str, datetime]:
         """Must return:
         * a unique hash as a string, representing the last version
-        * a datetime object corresponding to when the version dates
+        * a datetime object corresponding to when the version dates.
         """
         ...
 
     @abstractmethod
     def read_raw(self, hexsha: str, modified: datetime) -> Config:
-        """
-        Return the Config object that corresponds to the
+        """Return the Config object that corresponds to the
         specific hash
-        The `modified` parameter is just added as a attribute to the config
+        The `modified` parameter is just added as a attribute to the config.
 
         """
         ...
 
     def __init_subclass__(cls) -> None:
-        """
-        Keep a record of <scheme: class>
-        """
+        """Keep a record of <scheme: class>."""
         if cls.scheme in cls.__registry:
             raise TypeError(f"{cls.scheme=} is already define")
         cls.__registry[cls.scheme] = cls
@@ -94,19 +90,16 @@ def create(cls):
     def create_from_url(
         cls, *, backend_url: ConfigSourceUrl | Path | str
     ) -> ConfigSource:
-        """
-        Factory method to produce a concrete instance depending on
-        the backend URL scheme
+        """Factory method to produce a concrete instance depending on
+        the backend URL scheme.
 
         """
-
         url = TypeAdapter(ConfigSourceUrl).validate_python(str(backend_url))
         return cls.__registry[url.scheme](backend_url=url)
 
     def read_config(self) -> Config:
-        """
-        :raises:
-            git.exc.BadName if version does not exist
+        """:raises:
+        git.exc.BadName if version does not exist
         """
         hexsha, modified = self.latest_revision()
         return self.read_raw(hexsha, modified)
@@ -116,11 +109,10 @@ def clear_caches(self): ...
 
 
 class BaseGitConfigSource(ConfigSource):
-    """
-    Base class for the git based config source
+    """Base class for the git based config source
     The caching is based on 2 caches:
     * TTL to find the latest commit hashes
-    * LRU to keep in memory the last few versions
+    * LRU to keep in memory the last few versions.
 
     """
 
@@ -149,10 +141,7 @@ def latest_revision(self) -> tuple[str, datetime]:
 
     @cachedmethod(lambda self: self._read_raw_cache)
     def read_raw(self, hexsha: str, modified: datetime) -> Config:
-        """
-        :param: hexsha commit hash
-
-        """
+        """:param: hexsha commit hash"""
         logger.debug("Reading %s for %s with mtime %s", self, hexsha, modified)
         rev = self.repo.rev_parse(hexsha)
         blob = rev.tree / DEFAULT_CONFIG_FILE
@@ -168,9 +157,8 @@ def clear_caches(self):
 
 
 class LocalGitConfigSource(BaseGitConfigSource):
-    """
-    The configuration is stored on a local git repository
-    When running on multiple servers, the filesystem must be shared
+    """The configuration is stored on a local git repository
+    When running on multiple servers, the filesystem must be shared.
     """
 
     scheme = "git+file"
@@ -188,9 +176,7 @@ def __hash__(self):
 
 
 class RemoteGitConfigSource(BaseGitConfigSource):
-    """
-    Use a remote directory as a config source
-    """
+    """Use a remote directory as a config source."""
 
     scheme = "git+https"
 
@@ -217,7 +203,7 @@ def __hash__(self):
 
     @cachedmethod(lambda self: self._pull_cache)
     def _pull(self):
-        """Git pull from remote repo"""
+        """Git pull from remote repo."""
         print("CHRIS PULL")
         self.repo.remotes.origin.pull()
 

diracx-core/src/diracx/core/config/schema.py

@@ -24,7 +24,7 @@ class BaseModel(_BaseModel):
     @model_validator(mode="before")
     @classmethod
     def legacy_adaptor(cls, v):
-        """Applies transformations to interpret the legacy DIRAC CFG format"""
+        """Apply transformations to interpret the legacy DIRAC CFG format."""
         if not os.environ.get("DIRAC_COMPAT_ENABLE_CS_CONVERSION"):
             return v
         # If we're running with DIRAC_COMPAT_ENABLE_CS_CONVERSION set we apply

diracx-core/src/diracx/core/exceptions.py

@@ -19,23 +19,23 @@ class AuthorizationError(DiracError): ...
 
 
 class PendingAuthorizationError(AuthorizationError):
-    """Used to signal the device flow the authentication is still ongoing"""
+    """Used to signal the device flow the authentication is still ongoing."""
 
 
 class ExpiredFlowError(AuthorizationError):
-    """Used only for the Device Flow when the polling is expired"""
+    """Used only for the Device Flow when the polling is expired."""
 
 
 class ConfigurationError(DiracError):
-    """Used whenever we encounter a problem with the configuration"""
+    """Used whenever we encounter a problem with the configuration."""
 
 
 class BadConfigurationVersion(ConfigurationError):
-    """The requested version is not known"""
+    """The requested version is not known."""
 
 
 class InvalidQueryError(DiracError):
-    """It was not possible to build a valid database query from the given input"""
+    """It was not possible to build a valid database query from the given input."""
 
 
 class JobNotFound(Exception):

diracx-core/src/diracx/core/properties.py

@@ -1,5 +1,5 @@
-""" Just listing the possible Properties
-This module contains list of Properties that can be assigned to users and groups
+"""Just listing the possible Properties
+This module contains list of Properties that can be assigned to users and groups.
 """
 
 from __future__ import annotations

diracx-core/src/diracx/core/s3.py

@@ -54,7 +54,7 @@ async def generate_presigned_upload(
     size: int,
     validity_seconds: int,
 ) -> S3PresignedPostInfo:
-    """Generate a presigned URL and fields for uploading a file to S3
+    """Generate a presigned URL and fields for uploading a file to S3.
 
     The signature is restricted to only accept data with the given checksum and size.
     """
@@ -76,5 +76,5 @@ async def generate_presigned_upload(
 
 
 def b16_to_b64(hex_string: str) -> str:
-    """Convert hexadecimal encoded data to base64 encoded data"""
+    """Convert hexadecimal encoded data to base64 encoded data."""
     return base64.b64encode(base64.b16decode(hex_string.upper())).decode()

diracx-core/src/diracx/core/settings.py

@@ -31,7 +31,7 @@ def __init__(self, data: str):
 
 
 def _maybe_load_key_from_file(value: Any) -> Any:
-    """Load private keys from files if needed"""
+    """Load private keys from files if needed."""
     if isinstance(value, str) and not value.strip().startswith("-----BEGIN"):
         url = TypeAdapter(LocalFileUrl).validate_python(value)
         if not url.scheme == "file":

diracx-core/src/diracx/core/utils.py

@@ -12,7 +12,7 @@
 
 
 def dotenv_files_from_environment(prefix: str) -> list[str]:
-    """Get the sorted list of .env files to use for configuration"""
+    """Get the sorted list of .env files to use for configuration."""
     env_files = {}
     for key, value in os.environ.items():
         if match := re.fullmatch(rf"{prefix}(?:_(\d+))?", key):
@@ -21,7 +21,7 @@ def dotenv_files_from_environment(prefix: str) -> list[str]:
 
 
 def serialize_credentials(token_response: TokenResponse) -> str:
-    """Serialize DiracX client credentials to a string
+    """Serialize DiracX client credentials to a string.
 
     This method is separated from write_credentials to allow for DIRAC to be
     able to serialize credentials for inclusion in the proxy file.
@@ -38,7 +38,7 @@ def serialize_credentials(token_response: TokenResponse) -> str:
 
 
 def write_credentials(token_response: TokenResponse, *, location: Path | None = None):
-    """Write credentials received in dirax_preferences.credentials_path"""
+    """Write credentials received in dirax_preferences.credentials_path."""
     from diracx.core.preferences import get_diracx_preferences
 
     credentials_path = location or get_diracx_preferences().credentials_path

diracx-core/tests/test_s3.py

@@ -71,7 +71,7 @@ async def test_s3_object_exists(moto_s3):
 
 
 async def test_presigned_upload_moto(moto_s3):
-    """Test the presigned upload with moto
+    """Test the presigned upload with moto.
 
     This doesn't actually test the signature, see test_presigned_upload_minio
     """
@@ -94,7 +94,7 @@ async def test_presigned_upload_moto(moto_s3):
 
 @pytest.fixture(scope="function")
 async def minio_client(demo_urls):
-    """Create a S3 client that uses minio from the demo as backend"""
+    """Create a S3 client that uses minio from the demo as backend."""
     async with get_session().create_client(
         "s3",
         endpoint_url=demo_urls["minio"],
@@ -106,7 +106,7 @@ async def minio_client(demo_urls):
 
 @pytest.fixture(scope="function")
 async def test_bucket(minio_client):
-    """Create a test bucket that is cleaned up after the test session"""
+    """Create a test bucket that is cleaned up after the test session."""
     bucket_name = f"dirac-test-{secrets.token_hex(8)}"
     await minio_client.create_bucket(Bucket=bucket_name)
     yield bucket_name
@@ -131,7 +131,7 @@ async def test_bucket(minio_client):
 async def test_presigned_upload_minio(
     minio_client, test_bucket, content, checksum, size, expected_error
 ):
-    """Test the presigned upload with Minio
+    """Test the presigned upload with Minio.
 
     This is a more complete test that checks that the presigned upload works
     and is properly validated by Minio. This is not possible with moto as it

diracx-core/tests/test_secrets.py

@@ -8,7 +8,7 @@
 
 
 def compare_keys(key1, key2):
-    """Compare two keys by checking their public numebrs"""
+    """Compare two keys by checking their public numebrs."""
     assert key1.public_key().public_numbers() == key2.public_key().public_numbers()