♻️🔨 Is638/dynamic sidecar refactoring (round 1) (ITISFoundation#3147)

Author: pcrespov

.pre-commit-config.yaml

@@ -6,28 +6,35 @@ repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
     rev: v4.2.0
     hooks:
-      - id: trailing-whitespace
-      - id: end-of-file-fixer
-      - id: check-yaml
       - id: check-added-large-files
+        args: ["--maxkb=1024"]
       - id: check-merge-conflict
+      - id: check-yaml
+      - id: detect-private-key
+      - id: end-of-file-fixer
+      - id: trailing-whitespace
   # NOTE: Keep order as pyupgrade (will update code) then pycln (remove unused imports), then isort (sort them) and black (final formatting)
   - repo: https://github.com/asottile/pyupgrade
     rev: v2.34.0
     hooks:
       - id: pyupgrade
         args:
           - "--py39-plus"
+        name: upgrade code
   - repo: https://github.com/hadialqattan/pycln
     rev: v1.2.5
     hooks:
       - id: pycln
         args: [--all, --expand-stars]
+        name: prune imports
   - repo: https://github.com/PyCQA/isort
     rev: 5.10.1
     hooks:
       - id: isort
+        args: ["--profile", "black"]
+        name: sort imports
   - repo: https://github.com/psf/black
     rev: 22.3.0
     hooks:
       - id: black
+        name: format code

mypy.ini

@@ -19,8 +19,8 @@ disallow_any_generics = True
 check_untyped_defs = True
 no_implicit_reexport = True
 
-# for strict mypy: (this is the tricky one :-))
-disallow_untyped_defs = True
+# If True, it enforces things like `def __init__(self) -> CLASSNAME` or `def test_() -> None` which does not worth the effort
+disallow_untyped_defs = False
 
 # removes all the missing imports stuff from external libraries which is annoying to the least
 ignore_missing_imports = True

packages/pytest-simcore/src/pytest_simcore/docker_swarm.py

@@ -31,24 +31,16 @@
 # HELPERS --------------------------------------------------------------------------------
 
 
-class _NotInSwarmException(Exception):
-    pass
-
-
 class _ResourceStillNotRemoved(Exception):
     pass
 
 
-def _in_docker_swarm(
-    docker_client: docker.client.DockerClient, raise_error: bool = False
-) -> bool:
+def _is_docker_swarm_init(docker_client: docker.client.DockerClient) -> bool:
     try:
         docker_client.swarm.reload()
         inspect_result = docker_client.swarm.attrs
         assert type(inspect_result) == dict
     except APIError as error:
-        if raise_error:
-            raise _NotInSwarmException() from error
         return False
     return True
 
@@ -147,6 +139,7 @@ def _fetch_and_print_services(
 def docker_client() -> Iterator[docker.client.DockerClient]:
     client = docker.from_env()
     yield client
+    client.close()
 
 
 @pytest.fixture(scope="session")
@@ -164,14 +157,13 @@ def docker_swarm(
         wait=wait_fixed(2), stop=stop_after_delay(15), reraise=True
     ):
         with attempt:
-            if not _in_docker_swarm(docker_client):
+            if not _is_docker_swarm_init(docker_client):
                 print("--> initializing docker swarm...")
                 docker_client.swarm.init(advertise_addr=get_localhost_ip())
                 print("--> docker swarm initialized.")
-            # if still not in swarm, raise an error to try and initialize again
-            _in_docker_swarm(docker_client, raise_error=True)
 
-    assert _in_docker_swarm(docker_client) is True
+            # if still not in swarm, raise an error to try and initialize again
+            assert _is_docker_swarm_init(docker_client)
 
     yield
 
@@ -180,6 +172,8 @@ def docker_swarm(
         assert docker_client.swarm.leave(force=True)
         print("<-- docker swarm left.")
 
+    assert _is_docker_swarm_init(docker_client) is keep_docker_up
+
 
 @pytest.fixture(scope="module")
 def docker_stack(

packages/pytest-simcore/src/pytest_simcore/monkeypatch_extra.py

@@ -2,12 +2,18 @@
 # pylint: disable=unused-argument
 # pylint: disable=unused-variable
 
+import warnings
 from typing import Iterator
 
 import pytest
 from _pytest.fixtures import FixtureRequest
 from _pytest.monkeypatch import MonkeyPatch
 
+warnings.warn(
+    f"{__name__} is deprecated, we highly recommend to use pytest.monkeypatch at function-scope level."
+    "Large scopes lead to complex problems during tests",
+    DeprecationWarning,
+)
 # Some extras to overcome https://github.com/pytest-dev/pytest/issues/363
 # SEE https://github.com/pytest-dev/pytest/issues/363#issuecomment-289830794
 

packages/pytest-simcore/src/pytest_simcore/repository_paths.py

@@ -96,6 +96,13 @@ def pylintrc(osparc_simcore_root_dir: Path) -> Path:
 @pytest.fixture(scope="session")
 def project_slug_dir() -> Path:
     raise NotImplementedError("Override fixture in project's tests/conftest.py")
+    #
+    # Implementation example
+    #   folder = CURRENT_DIR.parent
+    #   assert folder.exists()
+    #   assert any(folder.glob("src/simcore_service_dynamic_sidecar"))
+    #   return folder
+    #
 
 
 @pytest.fixture(scope="session")

packages/settings-library/src/settings_library/utils_logging.py

@@ -4,8 +4,13 @@
 
 
 class MixinLoggingSettings:
+    """
+    USAGE example in packages/settings-library/tests/test_utils_logging.py::test_mixin_logging
+    """
+
     @classmethod
     def validate_log_level(cls, value: Any) -> str:
+        """Standard implementation for @validator("LOG_LEVEL")"""
         try:
             getattr(logging, value.upper())
         except AttributeError as err:

services/director-v2/tests/unit/conftest.py

@@ -67,7 +67,7 @@ def dynamic_service_create() -> DynamicServiceCreate:
     )
 
 
-@pytest.fixture(scope="session")
+@pytest.fixture
 def dynamic_sidecar_port() -> int:
     return 1222