From e76c3acf0da0fb135d02ff14c9561bdc5c07c806 Mon Sep 17 00:00:00 2001 From: Thomas Ubensee <34603111+tomuben@users.noreply.github.com> Date: Wed, 20 Nov 2024 17:46:46 -0300 Subject: [PATCH] Fixes from review --- .../lib/base/base_task.py | 2 +- .../lib/base/luigi_log_config.py | 26 ++++++++++-------- .../create/docker_image_analyze_task.py | 4 +-- .../lib/docker/networks/utils.py | 4 +-- .../abstract_spawn_test_environment.py | 27 ++++++++++--------- test/integration/conftest.py | 2 +- 6 files changed, 35 insertions(+), 30 deletions(-) diff --git a/exasol_integration_test_docker_environment/lib/base/base_task.py b/exasol_integration_test_docker_environment/lib/base/base_task.py index 42ff817cb..ed840cd01 100644 --- a/exasol_integration_test_docker_environment/lib/base/base_task.py +++ b/exasol_integration_test_docker_environment/lib/base/base_task.py @@ -153,7 +153,7 @@ def get_output_path(self) -> Path: def _get_output_path_for_job(self) -> Path: return Path(build_config().output_directory, - "jobs", self.job_id) # type: ignore + "jobs", self.job_id) def _extend_output_path(self): extension = self.extend_output_path() diff --git a/exasol_integration_test_docker_environment/lib/base/luigi_log_config.py b/exasol_integration_test_docker_environment/lib/base/luigi_log_config.py index 8022f6244..359d1fbdd 100644 --- a/exasol_integration_test_docker_environment/lib/base/luigi_log_config.py +++ b/exasol_integration_test_docker_environment/lib/base/luigi_log_config.py @@ -2,10 +2,12 @@ import os import tempfile from pathlib import Path -from typing import Optional, Callable, Generator +from typing import Optional, Callable, Generator, List, Any +from dataclasses import dataclass import jinja2 import logging + from exasol_integration_test_docker_environment.lib import PACKAGE_NAME from exasol_integration_test_docker_environment.lib.config.build_config import build_config @@ -40,22 +42,24 @@ def get_log_path(job_id: str) -> Path: log_path_dir.mkdir(parents=True, exist_ok=True) return log_path +@dataclass +class LogInfoStorage: + level : int + handlers: List[logging.Handler] + filters: List[Any] + propagate: bool @contextlib.contextmanager def restore_logger(logger_creator: Callable[[], logging.Logger]): before_logger = logger_creator() - logger_info = { - LOG_LEVEL: before_logger.level, - HANDLERS: list(before_logger.handlers), - FILTERS: list(before_logger.filters), - PROPAGATE: before_logger.propagate - } + logger_info_before = LogInfoStorage(level=before_logger.level, handlers=list(before_logger.handlers), + filters=list(before_logger.filters), propagate=before_logger.propagate) yield after_logger = logger_creator() - after_logger.level = logger_info[LOG_LEVEL] # type: ignore - after_logger.handlers = logger_info[HANDLERS] # type: ignore - after_logger.filters = logger_info[FILTERS] # type: ignore - after_logger.propagate = logger_info[PROPAGATE] # type: ignore + after_logger.level = logger_info_before.level + after_logger.handlers = logger_info_before.handlers + after_logger.filters = logger_info_before.filters + after_logger.propagate = logger_info_before.propagate @contextlib.contextmanager diff --git a/exasol_integration_test_docker_environment/lib/docker/images/create/docker_image_analyze_task.py b/exasol_integration_test_docker_environment/lib/docker/images/create/docker_image_analyze_task.py index 8eace9b67..376222075 100644 --- a/exasol_integration_test_docker_environment/lib/docker/images/create/docker_image_analyze_task.py +++ b/exasol_integration_test_docker_environment/lib/docker/images/create/docker_image_analyze_task.py @@ -137,8 +137,8 @@ def values_are_subclass_of_baseclass(self, task_classes) -> bool: return all(issubclass(value, DockerAnalyzeImageTask) for value in task_classes.values()) - def requires_tasks(self) -> Optional[Dict[str, Type["DockerAnalyzeImageTask"]]]: - pass + def requires_tasks(self) -> Dict[str, Type["DockerAnalyzeImageTask"]]: + return dict() def run_task(self): image_info_of_dependencies = self.get_values_from_futures(self.dependencies_futures) diff --git a/exasol_integration_test_docker_environment/lib/docker/networks/utils.py b/exasol_integration_test_docker_environment/lib/docker/networks/utils.py index 8b8adf5ed..03d4bea7c 100644 --- a/exasol_integration_test_docker_environment/lib/docker/networks/utils.py +++ b/exasol_integration_test_docker_environment/lib/docker/networks/utils.py @@ -1,10 +1,10 @@ import logging -from typing import Iterator, List +from typing import Iterable from exasol_integration_test_docker_environment.lib.docker import ContextDockerClient -def remove_docker_networks(networks: List[str]): +def remove_docker_networks(networks: Iterable[str]): """ Removes the given networks using docker API. """ diff --git a/exasol_integration_test_docker_environment/lib/test_environment/abstract_spawn_test_environment.py b/exasol_integration_test_docker_environment/lib/test_environment/abstract_spawn_test_environment.py index af70e8a4a..aa62a4c5a 100644 --- a/exasol_integration_test_docker_environment/lib/test_environment/abstract_spawn_test_environment.py +++ b/exasol_integration_test_docker_environment/lib/test_environment/abstract_spawn_test_environment.py @@ -68,7 +68,8 @@ def create_test_environment_info_in_test_container( shell_variables: ShellVariables, json: str, ): - test_container_name = test_environment_info.test_container_info.container_name # type: ignore + assert test_environment_info.test_container_info + test_container_name = test_environment_info.test_container_info.container_name with self._get_docker_client() as docker_client: test_container = docker_client.containers.get(test_container_name) self.logger.info(f"Create test environment info in test container '{test_container_name}' at '/'") @@ -102,31 +103,31 @@ def create_test_environment_info_in_test_container_and_on_host( json, ) - def _default_bridge_ip_address(self, test_environment_info) -> Optional[str]: - if test_environment_info.database_info.container_info is None: - return None - container_name = test_environment_info.database_info.container_info.container_name - with self._get_docker_client() as docker_client: - db_container = docker_client.containers.get(container_name) - return default_bridge_ip_address(db_container) + def _default_bridge_ip_address(self, test_environment_info) -> str: + if test_environment_info.database_info.container_info is not None: + container_name = test_environment_info.database_info.container_info.container_name + with self._get_docker_client() as docker_client: + db_container = docker_client.containers.get(container_name) + return default_bridge_ip_address(db_container) + raise RuntimeError("Could not find default bridge ip address") def collect_shell_variables(self, test_environment_info) -> ShellVariables: return ShellVariables.from_test_environment_info( - self._default_bridge_ip_address(test_environment_info), # type: ignore + self._default_bridge_ip_address(test_environment_info), test_environment_info, ) def _start_database(self, attempt) \ -> Generator[BaseTask, BaseTask, Tuple[DockerNetworkInfo, DatabaseInfo, bool, Optional[ContainerInfo]]]: - network_info = yield from self._create_network(attempt) # type: ignore - ssl_volume_info = None # type: ignore - if self.create_certificates: # type: ignore + network_info = yield from self._create_network(attempt) + ssl_volume_info = None + if self.create_certificates: ssl_volume_info = yield from self._create_ssl_certificates() # type: ignore database_info, test_container_info = yield from self._spawn_database_and_test_container(network_info, ssl_volume_info, attempt) # type: ignore is_database_ready = yield from self._wait_for_database(database_info, attempt) # type: ignore return network_info, database_info, is_database_ready, test_container_info # type: ignore - def _create_ssl_certificates(self) -> DockerVolumeInfo: # type: ignore + def _create_ssl_certificates(self) -> Generator: ssl_info_future = yield from self.run_dependencies(self.create_ssl_certificates()) ssl_info = self.get_values_from_future(ssl_info_future) return ssl_info # type: ignore diff --git a/test/integration/conftest.py b/test/integration/conftest.py index bb2d23870..096b6337b 100644 --- a/test/integration/conftest.py +++ b/test/integration/conftest.py @@ -67,7 +67,7 @@ def test_case(database): @contextlib.contextmanager # type: ignore def create_context( # type: ignore - name: Optional[str] = None, # type: ignore + name: Optional[str] = None, additional_parameters: Optional[List[str]] = None, ) -> SpawnedTestEnvironments: name = name if name else cli_isolation.name