Add unit tests, linting and tests CI/CD (#8)

* add lint and unit tests

.github/workflows/test.yaml

@@ -0,0 +1,9 @@
+name: Tests
+
+on:
+  pull_request:
+
+jobs:
+  unit-tests:
+    uses: canonical/operator-workflows/.github/workflows/test.yaml@cea2ac306b4f4c1475d73b1a4c766d62e5b1c8a9
+    secrets: inherit

.licenserc.yaml

@@ -0,0 +1,37 @@
+header:
+  license:
+    spdx-id: Apache-2.0
+    copyright-owner: Canonical Ltd.
+    copyright-year: 2024
+    content: |
+      Copyright [year] [owner]
+      See LICENSE file for licensing details.
+  paths:
+    - '**'
+  paths-ignore:
+    - '.github/**'
+    - '**/.gitkeep'
+    - '**/*.cfg'
+    - '**/*.conf'
+    - '**/*.j2'
+    - '**/*.json'
+    - '**/*.md'
+    - '**/*.rule'
+    - '**/*.tmpl'
+    - '**/*.txt'
+    - '**/*.jinja'
+    - '.codespellignore'
+    - '.dockerignore'
+    - '.flake8'
+    - '.jujuignore'
+    - '.gitignore'
+    - '.licenserc.yaml'
+    - '.trivyignore'
+    - '.woke.yaml'
+    - '.woke.yml'
+    - 'CODEOWNERS'
+    - 'icon.svg'
+    - 'LICENSE'
+    - 'trivy.yaml'
+    - 'lib/**'
+  comment: on-failure

.woke.yaml

@@ -0,0 +1,9 @@
+ignore_files:
+  # Ignore ingress charm library as it uses non compliant terminology:
+  # whitelist.
+  - lib/charms/data_platform_libs/v0/data_models.py
+  - lib/charms/data_platform_libs/v0/database_requires.py
+  - lib/charms/data_platform_libs/v0/s3.py
+rules:
+  # Ignore "master" - the database relation event received from the library.
+  - name: master

dev-requirements.txt

@@ -0,0 +1 @@
+tox==4.15.1

pyproject.toml

@@ -1,3 +1,11 @@
+# Copyright 2024 Canonical Ltd.
+# See LICENSE file for licensing details.
+
+[tool.bandit]
+exclude_dirs = ["/venv/"]
+[tool.bandit.assert_used]
+skips = ["*/*test.py", "*/test_*.py", "*tests/*.py"]
+
 # Testing tools configuration
 [tool.coverage.run]
 branch = true
@@ -11,36 +19,26 @@ log_cli_level = "INFO"
 
 # Formatting tools configuration
 [tool.black]
-line-length = 99
+line-length = 120
 target-version = ["py38"]
 
-# Linting tools configuration
-[tool.ruff]
-line-length = 99
-select = ["E", "W", "F", "C", "N", "D", "I001"]
-extend-ignore = [
-    "D203",
-    "D204",
-    "D213",
-    "D215",
-    "D400",
-    "D404",
-    "D406",
-    "D407",
-    "D408",
-    "D409",
-    "D413",
-]
-ignore = ["E501", "D107"]
-extend-exclude = ["__pycache__", "*.egg_info"]
-per-file-ignores = {"tests/*" = ["D100","D101","D102","D103","D104"]}
+[tool.isort]
+profile = "black"
 
-[tool.ruff.mccabe]
+# Linting tools configuration
+[tool.flake8]
+max-line-length = 120
+max-doc-length = 99
 max-complexity = 10
-
-[tool.codespell]
-skip = "build,lib,venv,icon.svg,.tox,.git,.mypy_cache,.ruff_cache,.coverage"
-
-[tool.pyright]
-include = ["src/**.py"]
-
+exclude = [".git", "__pycache__", ".tox", "build", "dist", "*.egg_info", "venv"]
+select = ["E", "W", "F", "C", "N", "R", "D", "H"]
+# Ignore W503, E501 because using black creates errors with this
+# Ignore D107 Missing docstring in __init__
+ignore = ["W503", "E501", "D107"]
+# D100, D101, D102, D103: Ignore missing docstrings in tests
+per-file-ignores = ["tests/*:D100,D101,D102,D103,D104"]
+docstring-convention = "google"
+# Check for properly formatted copyright header in each file
+copyright-check = "True"
+copyright-author = "Canonical Ltd."
+copyright-regexp = "Copyright\\s\\d{4}([-,]\\d{4})*\\s+%(author)s"

src/relations/airbyte_ui.py

@@ -5,10 +5,11 @@
 
 import logging
 
-from log import log_event_handler
 from ops import framework
 from ops.model import ActiveStatus
 
+from log import log_event_handler
+
 logger = logging.getLogger(__name__)
 
 
@@ -23,12 +24,8 @@ def __init__(self, charm):
         """
         super().__init__(charm, "airbyte-server")
         self.charm = charm
-        charm.framework.observe(
-            charm.on.airbyte_server_relation_joined, self._on_airbyte_server_relation_joined
-        )
-        charm.framework.observe(
-            charm.on.airbyte_server_relation_changed, self._on_airbyte_server_relation_joined
-        )
+        charm.framework.observe(charm.on.airbyte_server_relation_joined, self._on_airbyte_server_relation_joined)
+        charm.framework.observe(charm.on.airbyte_server_relation_changed, self._on_airbyte_server_relation_joined)
 
     @log_event_handler(logger)
     def _on_airbyte_server_relation_joined(self, event):

src/relations/minio.py

@@ -5,12 +5,17 @@
 
 import logging
 
-from charm_helpers import construct_svc_endpoint
 from charmed_kubeflow_chisme.exceptions import ErrorWithStatus
-from log import log_event_handler
 from ops import framework
 from ops.model import BlockedStatus, WaitingStatus
-from serialized_data_interface import NoCompatibleVersions, NoVersionsListed, get_interfaces
+from serialized_data_interface import (
+    NoCompatibleVersions,
+    NoVersionsListed,
+    get_interfaces,
+)
+
+from charm_helpers import construct_svc_endpoint
+from log import log_event_handler
 
 logger = logging.getLogger(__name__)
 
@@ -28,15 +33,9 @@ def __init__(self, charm):
         self.charm = charm
 
         # Handle minio relation.
-        charm.framework.observe(
-            charm.on.object_storage_relation_joined, self._on_object_storage_relation_changed
-        )
-        charm.framework.observe(
-            charm.on.object_storage_relation_changed, self._on_object_storage_relation_changed
-        )
-        charm.framework.observe(
-            charm.on.object_storage_relation_broken, self._on_object_storage_relation_broken
-        )
+        charm.framework.observe(charm.on.object_storage_relation_joined, self._on_object_storage_relation_changed)
+        charm.framework.observe(charm.on.object_storage_relation_changed, self._on_object_storage_relation_changed)
+        charm.framework.observe(charm.on.object_storage_relation_broken, self._on_object_storage_relation_broken)
 
     @log_event_handler(logger)
     def _on_object_storage_relation_changed(self, event):
@@ -90,22 +89,37 @@ def _on_object_storage_relation_broken(self, event) -> None:
         self.charm._update(event)
 
     def _get_interfaces(self):
-        """Retrieve interface object."""
+        """Retrieve interface object.
+
+        Returns:
+            list of charm interfaces.
+
+        Raises:
+            ErrorWithStatus: if an anticipated error occurs.
+        """
         try:
             charm = self.charm
-            # Hack: get_interfaces checks for peer relation which does not exist under requires/provides list in charmcraft.yaml
+            # Hack: get_interfaces checks for peer relation which does not exist under
+            # requires/provides list in charmcraft.yaml
             del charm.meta.relations["peer"]
             interfaces = get_interfaces(charm)
         except NoVersionsListed as err:
-            raise ErrorWithStatus(err, WaitingStatus)
+            raise ErrorWithStatus(err, WaitingStatus) from err
         except NoCompatibleVersions as err:
-            raise ErrorWithStatus(err, BlockedStatus)
+            raise ErrorWithStatus(err, BlockedStatus) from err
         return interfaces
 
     def _get_object_storage_data(self, interfaces):
         """Unpacks and returns the object-storage relation data.
 
-        Raises CheckFailedError if an anticipated error occurs.
+        Args:
+            interfaces: list of charm interfaces.
+
+        Returns:
+            object storage connection data.
+
+        Raises:
+            ErrorWithStatus: if an anticipated error occurs.
         """
         if not ((obj_storage := interfaces["object-storage"]) and obj_storage.get_data()):
             raise ErrorWithStatus("Waiting for object-storage relation data", WaitingStatus)
@@ -118,6 +132,6 @@ def _get_object_storage_data(self, interfaces):
                 f"Unexpected error unpacking object storage data - data format not "
                 f"as expected. Caught exception: '{str(e)}'",
                 BlockedStatus,
-            )
+            ) from e
 
         return obj_storage

src/relations/postgresql.py

@@ -6,11 +6,12 @@
 import logging
 
 from charms.data_platform_libs.v0.database_requires import DatabaseEvent
-from literals import DB_NAME
-from log import log_event_handler
 from ops import framework
 from ops.model import WaitingStatus
 
+from literals import DB_NAME
+from log import log_event_handler
+
 logger = logging.getLogger(__name__)
 
 

src/relations/s3.py

@@ -10,9 +10,10 @@
     CredentialsChangedEvent,
     CredentialsGoneEvent,
 )
-from log import log_event_handler
 from ops import framework
 
+from log import log_event_handler
+
 logger = logging.getLogger(__name__)
 
 
@@ -27,9 +28,7 @@ def __init__(self, charm):
         """
         super().__init__(charm, "s3")
         self.charm = charm
-        charm.framework.observe(
-            charm.s3_client.on.credentials_changed, self._on_s3_credentials_changed
-        )
+        charm.framework.observe(charm.s3_client.on.credentials_changed, self._on_s3_credentials_changed)
         charm.framework.observe(charm.s3_client.on.credentials_gone, self._on_s3_credentials_gone)
 
     @log_event_handler(logger)
@@ -81,9 +80,7 @@ def _retrieve_s3_parameters(self):
             "access-key",
             "secret-key",
         ]
-        missing_required_parameters = [
-            param for param in required_parameters if param not in s3_parameters
-        ]
+        missing_required_parameters = [param for param in required_parameters if param not in s3_parameters]
         if missing_required_parameters:
             logger.warning(
                 f"Missing required S3 parameters in relation with S3 integrator: {missing_required_parameters}"
@@ -104,9 +101,9 @@ def _retrieve_s3_parameters(self):
         # Clean up extra slash symbols to avoid issues on 3rd-party storages
         # like Ceph Object Gateway (radosgw).
         s3_parameters["endpoint"] = s3_parameters["endpoint"].rstrip("/")
-        s3_parameters["path"] = (
-            f'/{s3_parameters["path"].strip("/")}'  # The slash in the beginning is required by pgBackRest.
-        )
+        s3_parameters[
+            "path"
+        ] = f'/{s3_parameters["path"].strip("/")}'  # The slash in the beginning is required by pgBackRest.
         s3_parameters["bucket"] = s3_parameters["bucket"].strip("/")
 
         return s3_parameters, []

src/s3_helpers.py

@@ -15,33 +15,33 @@ class S3Client:
     """Client for S3 operations."""
 
     def __init__(self, s3_parameters):
-        """Construct.
+        """Initialize an S3 connection using the provided parameters.
 
         Args:
-            charm: The charm to attach the hooks to.
+            s3_parameters: S3 connection parameters.
+
+        Raises:
+            ValueError: If a session fails to be created.
         """
+        self.s3_parameters = s3_parameters
         endpoint = s3_parameters.get("endpoint")
         session = boto3.session.Session(
             aws_access_key_id=s3_parameters.get("access-key"),
             aws_secret_access_key=s3_parameters.get("secret-key"),
             region_name=s3_parameters.get("region"),  # Region can be optional for MinIO
         )
-        self.s3_parameters = s3_parameters
         try:
             self.s3_resource = session.resource("s3", endpoint_url=endpoint)
             self.s3_client = session.client("s3", endpoint_url=endpoint)
-        except ValueError as e:
-            logger.exception(
-                "Failed to create a session in region=%s.", s3_parameters.get("region")
-            )
-            raise e
+        except Exception as e:
+            logger.exception("Failed to create a session in region=%s.", s3_parameters.get("region"))
+            raise ValueError("Failed to create a session") from e
 
     def create_bucket_if_not_exists(self, bucket_name):
         """Create the S3 bucket if it does not exist.
 
         Args:
-            s3_parameters: S3 parameters fetched from the S3 integrator relation.
-            endpoint: S3 service endpoint.
+            bucket_name (str): name of bucket to create
 
         Raises:
             e (ValueError): if a session could not be created.
@@ -56,9 +56,7 @@ def create_bucket_if_not_exists(self, bucket_name):
         except ClientError as e:
             error_code = int(e.response["Error"]["Code"])
             if error_code == 404:
-                logger.warning(
-                    "Bucket %s doesn't exist or you don't have access to it.", bucket_name
-                )
+                logger.warning("Bucket %s doesn't exist or you don't have access to it.", bucket_name)
                 exists = False
             else:
                 logger.exception("Unexpected error: %s", e)
@@ -70,12 +68,16 @@ def create_bucket_if_not_exists(self, bucket_name):
                 s3_bucket.wait_until_exists()
                 logger.info("Created bucket '%s' in region=%s", bucket_name, region)
             except ClientError as error:
-                logger.exception(
-                    "Couldn't create bucket named '%s' in region=%s.", bucket_name, region
-                )
+                logger.exception("Couldn't create bucket named '%s' in region=%s.", bucket_name, region)
                 raise error
 
-    def set_bucket_lifecycle_policy(self, bucket, ttl):
+    def set_bucket_lifecycle_policy(self, bucket_name, ttl):
+        """Set lifecycle policy of bucket to purge files after a certain time.
+
+        Args:
+            bucket_name: Name of bucket.
+            ttl: Time to live of logs (in days).
+        """
         lifecycle_policy = {
             "Rules": [
                 {
@@ -86,6 +88,4 @@ def set_bucket_lifecycle_policy(self, bucket, ttl):
                 }
             ]
         }
-        self.s3_client.put_bucket_lifecycle_configuration(
-            Bucket=bucket, LifecycleConfiguration=lifecycle_policy
-        )
+        self.s3_client.put_bucket_lifecycle_configuration(Bucket=bucket_name, LifecycleConfiguration=lifecycle_policy)