From ac9a01c698e0de2bbdeeed1747c4536d4da92e1d Mon Sep 17 00:00:00 2001 From: Ivan Yordanov Date: Tue, 26 Sep 2023 15:19:13 +0200 Subject: [PATCH] Revert "Introduce ruff (#363)" This reverts commit 74dd3735 --- .github/ruff-matcher.json | 17 ---- .github/scripts/__init__.py | 0 .github/workflows/ci.yaml | 20 ++-- .pre-commit-config.yaml | 31 ++++-- README.md | 1 - docs/docs/schema/config.json | 2 +- docs/docs/schema/pipeline.json | 44 ++++----- hooks/__init__.py | 2 +- hooks/gen_docs/__init__.py | 2 +- hooks/gen_schema.py | 4 +- kpops/cli/registry.py | 27 +++-- kpops/component_handlers/__init__.py | 7 +- .../helm_wrapper/dry_run_handler.py | 2 +- kpops/component_handlers/helm_wrapper/helm.py | 44 ++++----- .../helm_wrapper/helm_diff.py | 5 +- .../component_handlers/helm_wrapper/model.py | 17 ++-- .../component_handlers/helm_wrapper/utils.py | 6 +- .../component_handlers/kafka_connect/model.py | 5 +- .../schema_handler/schema_handler.py | 25 ++--- .../schema_handler/schema_provider.py | 5 +- kpops/component_handlers/utils/exception.py | 6 +- .../base_defaults_component.py | 19 ++-- kpops/components/base_components/kafka_app.py | 14 +-- .../base_components/kafka_connector.py | 39 ++++---- .../base_components/kubernetes_app.py | 26 +++-- .../base_components/models/from_section.py | 11 +-- .../base_components/models/to_section.py | 11 +-- .../streams_bootstrap/producer/model.py | 4 +- .../producer/producer_app.py | 7 +- .../streams_bootstrap/streams/model.py | 13 ++- .../streams_bootstrap/streams/streams_app.py | 6 +- kpops/utils/dict_differ.py | 18 ++-- kpops/utils/dict_ops.py | 20 ++-- kpops/utils/docstring.py | 6 +- kpops/utils/environment.py | 2 +- kpops/utils/gen_schema.py | 22 ++--- kpops/utils/yaml_loading.py | 7 +- pyproject.toml | 98 +------------------ setup.cfg | 19 ++++ .../snapshots/snap_test_schema_generation.py | 16 +-- tests/cli/test_schema_generation.py | 13 ++- tests/compiler/test_pipeline_name.py | 42 ++++---- .../helm_wrapper/test_dry_run_handler.py | 4 +- .../helm_wrapper/test_helm_wrapper.py | 11 ++- .../test_base_defaults_component.py | 4 +- tests/components/test_kafka_app.py | 4 +- tests/components/test_kafka_connector.py | 14 +-- tests/components/test_kafka_sink_connector.py | 4 +- .../components/test_kafka_source_connector.py | 2 +- tests/components/test_kubernetes_app.py | 18 ++-- tests/components/test_producer_app.py | 6 +- tests/components/test_streams_app.py | 14 +-- tests/pipeline/test_components/components.py | 5 +- tests/pipeline/test_pipeline.py | 3 +- tests/pipeline/test_template.py | 2 +- tests/utils/test_environment.py | 4 +- 56 files changed, 339 insertions(+), 441 deletions(-) delete mode 100644 .github/ruff-matcher.json delete mode 100644 .github/scripts/__init__.py create mode 100644 setup.cfg diff --git a/.github/ruff-matcher.json b/.github/ruff-matcher.json deleted file mode 100644 index bc3b10738..000000000 --- a/.github/ruff-matcher.json +++ /dev/null @@ -1,17 +0,0 @@ -{ - "problemMatcher": [ - { - "owner": "ruff", - "pattern": [ - { - "regexp": "^(.*):(\\d+):(\\d+):\\s([\\da-zA-Z]+)\\s(.*)$", - "file": 1, - "line": 2, - "column": 3, - "code": 4, - "message": 5 - } - ] - } - ] -} diff --git a/.github/scripts/__init__.py b/.github/scripts/__init__.py deleted file mode 100644 index e69de29bb..000000000 diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index fe87e8436..baa091133 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -35,16 +35,11 @@ jobs: - name: Install dependencies run: poetry install --no-interaction - - name: Lint (ruff) - shell: bash - run: | - if [[ "$RUNNER_OS" == "Linux" && "${{ matrix.python-version }}" == "3.10" ]] - then - echo "::add-matcher::.github/ruff-matcher.json" - poetry run ruff check . --config pyproject.toml --output-format text --no-fix - else - poetry run pre-commit run ruff --all-files --show-diff-on-failure - fi; + - name: Lint (flake8) + run: poetry run pre-commit run flake8 --all-files --show-diff-on-failure + + - name: Order of imports (isort) + run: poetry run pre-commit run isort --all-files --show-diff-on-failure - name: Formatting (black) run: poetry run pre-commit run black --all-files --show-diff-on-failure @@ -64,6 +59,11 @@ jobs: - name: Generate pipeline definitions run: poetry run pre-commit run gen-docs-components --all-files --show-diff-on-failure + # TODO: enable when PEP 604 incompatibilty is in typer is resolved https://github.com/tiangolo/typer/issues/348 + # See https://github.com/tiangolo/typer/pull/522 + # - name: Syntax (pyupgrade) + # run: poetry run pre-commit run --hook-stage manual pyupgrade --all-files + - name: Test run: poetry run pytest tests diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8c709b20a..59639ac35 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,14 +1,12 @@ repos: + - repo: https://github.com/pycqa/isort + rev: 5.12.0 + hooks: + - id: isort + args: ["--settings", "setup.cfg"] + exclude: ^tests/.*snapshots/ - repo: local hooks: - - id: ruff - name: ruff - entry: ruff check . - args: [ --config, pyproject.toml, --fix, --show-fixes, --exit-non-zero-on-fix ] - language: system - types_or: [python] - require_serial: true # run once for all files - pass_filenames: false - id: black name: black entry: black @@ -16,6 +14,14 @@ repos: types_or: [python, pyi] require_serial: true # run once for all files exclude: ^tests/.*snapshots/ + - repo: https://github.com/pycqa/flake8 + rev: 5.0.4 + hooks: + - id: flake8 + args: ["--config", "setup.cfg"] + exclude: ^tests/.*snapshots/ + - repo: local + hooks: - id: pyright name: pyright entry: pyright @@ -23,6 +29,15 @@ repos: types: [python] require_serial: true # run once for all files exclude: ^tests/.*snapshots/ + - repo: https://github.com/asottile/pyupgrade + rev: v3.1.0 + hooks: + - id: pyupgrade + stages: [manual] + args: ["--py310-plus"] + exclude: ^tests/.*snapshots/ + - repo: local + hooks: - id: gen-schema name: gen-schema entry: python hooks/gen_schema.py diff --git a/README.md b/README.md index 9dd25fd9c..9d2aaca2e 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,6 @@ [![Build status](https://github.com/bakdata/kpops/actions/workflows/ci.yaml/badge.svg)](https://github.com/bakdata/kpops/actions/workflows/ci.yaml) [![pypi](https://img.shields.io/pypi/v/kpops.svg)](https://pypi.org/project/kpops) [![versions](https://img.shields.io/pypi/pyversions/kpops.svg)](https://github.com/bakdata/kpops) -[![Ruff](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/astral-sh/ruff/main/assets/badge/v2.json)](https://github.com/astral-sh/ruff) [![license](https://img.shields.io/github/license/bakdata/kpops.svg)](https://github.com/bakdata/kpops/blob/main/LICENSE) ## Key features diff --git a/docs/docs/schema/config.json b/docs/docs/schema/config.json index a2f18eb6b..b77b4e850 100644 --- a/docs/docs/schema/config.json +++ b/docs/docs/schema/config.json @@ -2,7 +2,7 @@ "$ref": "#/definitions/PipelineConfig", "definitions": { "HelmConfig": { - "description": "Global Helm configuration.", + "description": "Global Helm configuration", "properties": { "api_version": { "description": "Kubernetes API version used for Capabilities.APIVersions", diff --git a/docs/docs/schema/pipeline.json b/docs/docs/schema/pipeline.json index 7e77b0ddd..9695ec9a2 100644 --- a/docs/docs/schema/pipeline.json +++ b/docs/docs/schema/pipeline.json @@ -2,7 +2,7 @@ "definitions": { "FromSection": { "additionalProperties": false, - "description": "Holds multiple input topics.", + "description": "Holds multiple input topics", "properties": { "components": { "additionalProperties": { @@ -28,7 +28,7 @@ }, "FromTopic": { "additionalProperties": false, - "description": "Input topic.", + "description": "Input topic", "properties": { "role": { "description": "Custom identifier belonging to a topic; define only if `type` is `pattern` or `None`", @@ -48,7 +48,7 @@ "type": "object" }, "HelmRepoConfig": { - "description": "Helm repository configuration.", + "description": "Helm repository configuration", "properties": { "repo_auth_flags": { "allOf": [ @@ -85,7 +85,7 @@ "type": "object" }, "InputTopicTypes": { - "description": "Input topic types.\n\nINPUT (input topic), PATTERN (extra-topic-pattern or input-topic-pattern)", + "description": "Input topic types\n\nINPUT (input topic), PATTERN (extra-topic-pattern or input-topic-pattern)", "enum": [ "input", "pattern" @@ -97,7 +97,7 @@ "additionalProperties": { "type": "string" }, - "description": "Settings specific to Kafka Connectors.", + "description": "Settings specific to Kafka Connectors", "properties": { "connector.class": { "title": "Connector.Class", @@ -111,7 +111,7 @@ "type": "object" }, "KafkaSinkConnector": { - "description": "Kafka sink connector model.", + "description": "Kafka sink connector model", "properties": { "app": { "allOf": [ @@ -183,7 +183,7 @@ }, "type": { "default": "kafka-sink-connector", - "description": "Kafka sink connector model.", + "description": "Kafka sink connector model", "enum": [ "kafka-sink-connector" ], @@ -206,7 +206,7 @@ "type": "object" }, "KafkaSourceConnector": { - "description": "Kafka source connector model.", + "description": "Kafka source connector model", "properties": { "app": { "allOf": [ @@ -283,7 +283,7 @@ }, "type": { "default": "kafka-source-connector", - "description": "Kafka source connector model.", + "description": "Kafka source connector model", "enum": [ "kafka-source-connector" ], @@ -384,13 +384,13 @@ "type": "object" }, "KubernetesAppConfig": { - "description": "Settings specific to Kubernetes Apps.", + "description": "Settings specific to Kubernetes Apps", "properties": {}, "title": "KubernetesAppConfig", "type": "object" }, "OutputTopicTypes": { - "description": "Types of output topic.\n\nOUTPUT (output topic), ERROR (error topic)", + "description": "Types of output topic\n\nOUTPUT (output topic), ERROR (error topic)", "enum": [ "output", "error" @@ -399,7 +399,7 @@ "type": "string" }, "ProducerApp": { - "description": "Producer component.\nThis producer holds configuration to use as values for the streams bootstrap producer helm chart. Note that the producer does not support error topics.", + "description": "Producer component\nThis producer holds configuration to use as values for the streams bootstrap producer helm chart. Note that the producer does not support error topics.", "properties": { "app": { "allOf": [ @@ -462,7 +462,7 @@ }, "type": { "default": "producer-app", - "description": "Producer component.\nThis producer holds configuration to use as values for the streams bootstrap producer helm chart. Note that the producer does not support error topics.", + "description": "Producer component\nThis producer holds configuration to use as values for the streams bootstrap producer helm chart. Note that the producer does not support error topics.", "enum": [ "producer-app" ], @@ -485,7 +485,7 @@ "type": "object" }, "ProducerStreamsConfig": { - "description": "Kafka Streams settings specific to Producer.", + "description": "Kafka Streams settings specific to Producer", "properties": { "brokers": { "description": "Brokers", @@ -519,7 +519,7 @@ "type": "object" }, "ProducerValues": { - "description": "Settings specific to producers.", + "description": "Settings specific to producers", "properties": { "nameOverride": { "description": "Override name with this value", @@ -543,7 +543,7 @@ "type": "object" }, "RepoAuthFlags": { - "description": "Authorisation-related flags for `helm repo`.", + "description": "Authorisation-related flags for `helm repo`", "properties": { "ca_file": { "description": "Path to CA bundle file to verify certificates of HTTPS-enabled servers", @@ -578,7 +578,7 @@ "type": "object" }, "StreamsApp": { - "description": "StreamsApp component that configures a streams bootstrap app.", + "description": "StreamsApp component that configures a streams bootstrap app", "properties": { "app": { "allOf": [ @@ -645,7 +645,7 @@ }, "type": { "default": "streams-app", - "description": "StreamsApp component that configures a streams bootstrap app.", + "description": "StreamsApp component that configures a streams bootstrap app", "enum": [ "streams-app" ], @@ -668,7 +668,7 @@ "type": "object" }, "StreamsAppAutoScaling": { - "description": "Kubernetes Event-driven Autoscaling config.", + "description": "Kubernetes Event-driven Autoscaling config", "properties": { "consumerGroup": { "description": "Name of the consumer group used for checking the offset on the topic and processing the related lag.", @@ -771,7 +771,7 @@ "type": "object" }, "StreamsConfig": { - "description": "Streams Bootstrap streams section.", + "description": "Streams Bootstrap streams section", "properties": { "brokers": { "description": "Brokers", @@ -854,7 +854,7 @@ "type": "object" }, "ToSection": { - "description": "Holds multiple output topics.", + "description": "Holds multiple output topics", "properties": { "models": { "additionalProperties": { @@ -880,7 +880,7 @@ }, "TopicConfig": { "additionalProperties": false, - "description": "Configure an output topic.", + "description": "Configure an output topic", "properties": { "configs": { "additionalProperties": { diff --git a/hooks/__init__.py b/hooks/__init__.py index ef17ce38a..0ae8ea143 100644 --- a/hooks/__init__.py +++ b/hooks/__init__.py @@ -1,4 +1,4 @@ -"""KPOps pre-commit hooks.""" +"""KPOps pre-commit hooks""" from pathlib import Path PATH_ROOT = Path(__file__).parents[1] diff --git a/hooks/gen_docs/__init__.py b/hooks/gen_docs/__init__.py index 5a0d63a28..5052e8077 100644 --- a/hooks/gen_docs/__init__.py +++ b/hooks/gen_docs/__init__.py @@ -5,7 +5,7 @@ class IterableStrEnum(str, Enum): - """Polyfill that also introduces dict-like behavior. + """Polyfill that also introduces dict-like behavior Introduces constructors that return a ``Iterator`` object either containing all items, only their names or their values. diff --git a/hooks/gen_schema.py b/hooks/gen_schema.py index 7d6b99f2e..8fc24f938 100644 --- a/hooks/gen_schema.py +++ b/hooks/gen_schema.py @@ -1,4 +1,4 @@ -"""Generates the stock KPOps editor integration schemas.""" +"""Generates the stock KPOps editor integration schemas""" from contextlib import redirect_stdout from io import StringIO from pathlib import Path @@ -10,7 +10,7 @@ def gen_schema(scope: SchemaScope): - """Generate the specified schema and save it to a file. + """Generates the specified schema and saves it to a file. The file is located in docs/docs/schema and is named ``.json`` diff --git a/kpops/cli/registry.py b/kpops/cli/registry.py index a97e2cd91..410aa1be5 100644 --- a/kpops/cli/registry.py +++ b/kpops/cli/registry.py @@ -2,24 +2,22 @@ import importlib import inspect +import os import sys +from collections.abc import Iterator from dataclasses import dataclass, field -from pathlib import Path -from typing import TYPE_CHECKING, TypeVar +from typing import TypeVar from kpops import __name__ from kpops.cli.exception import ClassNotFoundError from kpops.components.base_components.pipeline_component import PipelineComponent -if TYPE_CHECKING: - from collections.abc import Iterator - KPOPS_MODULE = __name__ + "." T = TypeVar("T") ClassDict = dict[str, type[T]] # type -> class -sys.path.append(str(Path.cwd())) +sys.path.append(os.getcwd()) @dataclass @@ -29,9 +27,9 @@ class Registry: _classes: ClassDict[PipelineComponent] = field(default_factory=dict, init=False) def find_components(self, module_name: str) -> None: - """Find all PipelineComponent subclasses in module. - - :param module_name: name of the python module. + """ + Find all PipelineComponent subclasses in module + :param module_name: name of the python module """ for _class in _find_classes(module_name, PipelineComponent): self._classes[_class.type] = _class @@ -39,16 +37,17 @@ def find_components(self, module_name: str) -> None: def __getitem__(self, component_type: str) -> type[PipelineComponent]: try: return self._classes[component_type] - except KeyError as ke: - msg = f"Could not find a component of type {component_type}" - raise ClassNotFoundError(msg) from ke + except KeyError: + raise ClassNotFoundError( + f"Could not find a component of type {component_type}" + ) def find_class(module_name: str, baseclass: type[T]) -> type[T]: try: return next(_find_classes(module_name, baseclass)) - except StopIteration as e: - raise ClassNotFoundError from e + except StopIteration: + raise ClassNotFoundError def _find_classes(module_name: str, baseclass: type[T]) -> Iterator[type[T]]: diff --git a/kpops/component_handlers/__init__.py b/kpops/component_handlers/__init__.py index fa296a574..988ca7ee7 100644 --- a/kpops/component_handlers/__init__.py +++ b/kpops/component_handlers/__init__.py @@ -2,10 +2,11 @@ from typing import TYPE_CHECKING +from kpops.component_handlers.kafka_connect.kafka_connect_handler import ( + KafkaConnectHandler, +) + if TYPE_CHECKING: - from kpops.component_handlers.kafka_connect.kafka_connect_handler import ( - KafkaConnectHandler, - ) from kpops.component_handlers.schema_handler.schema_handler import SchemaHandler from kpops.component_handlers.topic.handler import TopicHandler diff --git a/kpops/component_handlers/helm_wrapper/dry_run_handler.py b/kpops/component_handlers/helm_wrapper/dry_run_handler.py index 2d28957b7..8e260f7df 100644 --- a/kpops/component_handlers/helm_wrapper/dry_run_handler.py +++ b/kpops/component_handlers/helm_wrapper/dry_run_handler.py @@ -11,7 +11,7 @@ def __init__(self, helm: Helm, helm_diff: HelmDiff, namespace: str) -> None: self.namespace = namespace def print_helm_diff(self, stdout: str, helm_release_name: str, log: Logger) -> None: - """Print the diff of the last and current release of this component. + """Print the diff of the last and current release of this component :param stdout: The output of a Helm command that installs or upgrades the release :param helm_release_name: The Helm release name diff --git a/kpops/component_handlers/helm_wrapper/helm.py b/kpops/component_handlers/helm_wrapper/helm.py index b1b101b41..2ad3f5f01 100644 --- a/kpops/component_handlers/helm_wrapper/helm.py +++ b/kpops/component_handlers/helm_wrapper/helm.py @@ -4,7 +4,8 @@ import re import subprocess import tempfile -from typing import TYPE_CHECKING +from collections.abc import Iterator +from typing import Iterable import yaml @@ -19,9 +20,6 @@ Version, ) -if TYPE_CHECKING: - from collections.abc import Iterable, Iterator - log = logging.getLogger("Helm") @@ -31,17 +29,16 @@ def __init__(self, helm_config: HelmConfig) -> None: self._debug = helm_config.debug self._version = self.get_version() if self._version.major != 3: - msg = f"The supported Helm version is 3.x.x. The current Helm version is {self._version.major}.{self._version.minor}.{self._version.patch}" - raise RuntimeError(msg) + raise RuntimeError( + f"The supported Helm version is 3.x.x. The current Helm version is {self._version.major}.{self._version.minor}.{self._version.patch}" + ) def add_repo( self, repository_name: str, repository_url: str, - repo_auth_flags: RepoAuthFlags | None = None, + repo_auth_flags: RepoAuthFlags = RepoAuthFlags(), ) -> None: - if repo_auth_flags is None: - repo_auth_flags = RepoAuthFlags() command = [ "helm", "repo", @@ -53,7 +50,7 @@ def add_repo( try: self.__execute(command) - except (ReleaseNotFoundException, RuntimeError) as e: + except Exception as e: if ( len(e.args) == 1 and re.match( @@ -62,9 +59,9 @@ def add_repo( ) is not None ): - log.exception(f"Could not add repository {repository_name}.") + log.error(f"Could not add repository {repository_name}. {e}") else: - raise + raise e if self._version.minor > 7: self.__execute(["helm", "repo", "update", repository_name]) @@ -78,11 +75,9 @@ def upgrade_install( dry_run: bool, namespace: str, values: dict, - flags: HelmUpgradeInstallFlags | None = None, + flags: HelmUpgradeInstallFlags = HelmUpgradeInstallFlags(), ) -> str: - """Prepare and execute the `helm upgrade --install` command.""" - if flags is None: - flags = HelmUpgradeInstallFlags() + """Prepares and executes the `helm upgrade --install` command""" with tempfile.NamedTemporaryFile("w") as values_file: yaml.safe_dump(values, values_file) @@ -108,7 +103,7 @@ def uninstall( release_name: str, dry_run: bool, ) -> str | None: - """Prepare and execute the helm uninstall command.""" + """Prepares and executes the helm uninstall command""" command = [ "helm", "uninstall", @@ -131,7 +126,7 @@ def template( chart: str, namespace: str, values: dict, - flags: HelmTemplateFlags | None = None, + flags: HelmTemplateFlags = HelmTemplateFlags(), ) -> str: """From HELM: Render chart templates locally and display the output. @@ -146,8 +141,6 @@ def template( :param flags: the flags to be set for `helm template`, defaults to HelmTemplateFlags() :return: the output of `helm template` """ - if flags is None: - flags = HelmTemplateFlags() with tempfile.NamedTemporaryFile("w") as values_file: yaml.safe_dump(values, values_file) command = [ @@ -184,8 +177,9 @@ def get_version(self) -> Version: short_version = self.__execute(command) version_match = re.search(r"^v(\d+(?:\.\d+){0,2})", short_version) if version_match is None: - msg = f"Could not parse the Helm version.\n\nHelm output:\n{short_version}" - raise RuntimeError(msg) + raise RuntimeError( + f"Could not parse the Helm version.\n\nHelm output:\n{short_version}" + ) version = map(int, version_match.group(1).split(".")) return Version(*version) @@ -212,8 +206,8 @@ def __execute(self, command: list[str]) -> str: log.debug(f"Executing {' '.join(command)}") process = subprocess.run( command, - check=True, - capture_output=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, text=True, ) Helm.parse_helm_command_stderr_output(process.stderr) @@ -234,7 +228,7 @@ def parse_helm_command_stderr_output(stderr: str) -> None: for line in stderr.splitlines(): lower = line.lower() if "release: not found" in lower: - raise ReleaseNotFoundException + raise ReleaseNotFoundException() elif "error" in lower: raise RuntimeError(stderr) elif "warning" in lower: diff --git a/kpops/component_handlers/helm_wrapper/helm_diff.py b/kpops/component_handlers/helm_wrapper/helm_diff.py index 26de5613a..e778a7df2 100644 --- a/kpops/component_handlers/helm_wrapper/helm_diff.py +++ b/kpops/component_handlers/helm_wrapper/helm_diff.py @@ -1,5 +1,6 @@ import logging -from collections.abc import Iterable, Iterator +from collections.abc import Iterator +from typing import Iterable from kpops.component_handlers.helm_wrapper.model import HelmDiffConfig, HelmTemplate from kpops.utils.dict_differ import Change, render_diff @@ -16,7 +17,7 @@ def calculate_changes( current_release: Iterable[HelmTemplate], new_release: Iterable[HelmTemplate], ) -> Iterator[Change[dict]]: - """Compare 2 releases and generate a Change object for each difference. + """Compare 2 releases and generate a Change object for each difference :param current_release: Iterable containing HelmTemplate objects for the current release :param new_release: Iterable containing HelmTemplate objects for the new release diff --git a/kpops/component_handlers/helm_wrapper/model.py b/kpops/component_handlers/helm_wrapper/model.py index af21abb3f..a8aaf8906 100644 --- a/kpops/component_handlers/helm_wrapper/model.py +++ b/kpops/component_handlers/helm_wrapper/model.py @@ -1,6 +1,6 @@ -from collections.abc import Iterator from dataclasses import dataclass from pathlib import Path +from typing import Iterator import yaml from pydantic import BaseConfig, BaseModel, Extra, Field @@ -20,7 +20,7 @@ class HelmDiffConfig(BaseModel): class RepoAuthFlags(BaseModel): - """Authorisation-related flags for `helm repo`. + """Authorisation-related flags for `helm repo` :param username: Username, defaults to None :param password: Password, defaults to None @@ -65,7 +65,7 @@ def to_command(self) -> list[str]: class HelmRepoConfig(BaseModel): - """Helm repository configuration. + """Helm repository configuration :param repository_name: Name of the Helm repository :param url: URL to the Helm repository @@ -85,7 +85,7 @@ class Config(DescConfig): class HelmConfig(BaseModel): - """Global Helm configuration. + """Global Helm configuration :param context: Name of kubeconfig context (`--kube-context`) :param debug: Run Helm in Debug mode @@ -180,8 +180,7 @@ def parse_source(source: str) -> str: # Source: chart/templates/serviceaccount.yaml """ if not source.startswith(HELM_SOURCE_PREFIX): - msg = "Not a valid Helm template source" - raise ParseError(msg) + raise ParseError("Not a valid Helm template source") return source.removeprefix(HELM_SOURCE_PREFIX).strip() @classmethod @@ -206,9 +205,9 @@ def __iter__(self) -> Iterator[str]: @property def manifest(self) -> str: - """Reads the manifest section of Helm stdout. - - `helm upgrade --install` output message contains three sections in the following order: + """ + Reads the manifest section of Helm stdout. `helm upgrade --install` output message contains three sections + in the following order: - HOOKS - MANIFEST diff --git a/kpops/component_handlers/helm_wrapper/utils.py b/kpops/component_handlers/helm_wrapper/utils.py index 7ad76b93a..d39536041 100644 --- a/kpops/component_handlers/helm_wrapper/utils.py +++ b/kpops/component_handlers/helm_wrapper/utils.py @@ -7,11 +7,11 @@ def trim_release_name(name: str, suffix: str = "") -> str: - """Trim Helm release name while preserving suffix. - + """ + Trim Helm release name while preserving suffix. :param name: The release name including optional suffix :param suffix: The release suffix to preserve - :return: Truncated release name. + :return: Truncated release name """ if len(name) > RELEASE_NAME_MAX_LEN: new_name = name[: (RELEASE_NAME_MAX_LEN - len(suffix))] + suffix diff --git a/kpops/component_handlers/kafka_connect/model.py b/kpops/component_handlers/kafka_connect/model.py index e83e33e5d..9feed448f 100644 --- a/kpops/component_handlers/kafka_connect/model.py +++ b/kpops/component_handlers/kafka_connect/model.py @@ -13,7 +13,7 @@ class KafkaConnectorType(str, Enum): class KafkaConnectorConfig(BaseModel): - """Settings specific to Kafka Connectors.""" + """Settings specific to Kafka Connectors""" connector_class: str name: str = Field(default=..., hidden_from_schema=True) @@ -31,8 +31,7 @@ def schema_extra(cls, schema: dict[str, Any], model: type[BaseModel]) -> None: @validator("connector_class") def connector_class_must_contain_dot(cls, connector_class: str) -> str: if "." not in connector_class: - msg = f"Invalid connector class {connector_class}" - raise ValueError(msg) + raise ValueError(f"Invalid connector class {connector_class}") return connector_class @property diff --git a/kpops/component_handlers/schema_handler/schema_handler.py b/kpops/component_handlers/schema_handler/schema_handler.py index 8bb046763..3a408a519 100644 --- a/kpops/component_handlers/schema_handler/schema_handler.py +++ b/kpops/component_handlers/schema_handler/schema_handler.py @@ -3,23 +3,20 @@ import json import logging from functools import cached_property -from typing import TYPE_CHECKING from schema_registry.client import AsyncSchemaRegistryClient from schema_registry.client.schema import AvroSchema from kpops.cli.exception import ClassNotFoundError +from kpops.cli.pipeline_config import PipelineConfig from kpops.cli.registry import find_class from kpops.component_handlers.schema_handler.schema_provider import ( Schema, SchemaProvider, ) +from kpops.components.base_components.models.to_section import ToSection from kpops.utils.colorify import greenify, magentaify -if TYPE_CHECKING: - from kpops.cli.pipeline_config import PipelineConfig - from kpops.components.base_components.models.to_section import ToSection - log = logging.getLogger("SchemaHandler") @@ -32,13 +29,16 @@ def __init__(self, url: str, components_module: str | None): def schema_provider(self) -> SchemaProvider: try: if not self.components_module: - msg = f"The Schema Registry URL is set but you haven't specified the component module path. Please provide a valid component module path where your {SchemaProvider.__name__} implementation exists." - raise ValueError(msg) + raise ValueError( + f"The Schema Registry URL is set but you haven't specified the component module path. Please provide a valid component module path where your {SchemaProvider.__name__} implementation exists." + ) schema_provider_class = find_class(self.components_module, SchemaProvider) return schema_provider_class() # pyright: ignore[reportGeneralTypeIssues] - except ClassNotFoundError as e: - msg = f"No schema provider found in components module {self.components_module}. Please implement the abstract method in {SchemaProvider.__module__}.{SchemaProvider.__name__}." - raise ValueError(msg) from e + except ClassNotFoundError: + raise ValueError( + f"No schema provider found in components module {self.components_module}. " + f"Please implement the abstract method in {SchemaProvider.__module__}.{SchemaProvider.__name__}." + ) @classmethod def load_schema_handler( @@ -148,8 +148,9 @@ async def __check_compatibility( if isinstance(schema, AvroSchema) else str(schema) ) - msg = f"Schema is not compatible for {subject} and model {schema_class}. \n {json.dumps(schema_str, indent=4)}" - raise Exception(msg) + raise Exception( + f"Schema is not compatible for {subject} and model {schema_class}. \n {json.dumps(schema_str, indent=4)}" + ) else: log.debug( f"Schema Submission: schema was already submitted for the subject {subject} as version {registered_version.schema}. Therefore, the specified schema must be compatible." diff --git a/kpops/component_handlers/schema_handler/schema_provider.py b/kpops/component_handlers/schema_handler/schema_provider.py index 0c0423a40..2b93bf943 100644 --- a/kpops/component_handlers/schema_handler/schema_provider.py +++ b/kpops/component_handlers/schema_handler/schema_provider.py @@ -1,12 +1,11 @@ from __future__ import annotations from abc import ABC, abstractmethod -from typing import TYPE_CHECKING, TypeAlias +from typing import TypeAlias from schema_registry.client.schema import AvroSchema, JsonSchema -if TYPE_CHECKING: - from kpops.components.base_components.models import ModelName, ModelVersion +from kpops.components.base_components.models import ModelName, ModelVersion Schema: TypeAlias = AvroSchema | JsonSchema diff --git a/kpops/component_handlers/utils/exception.py b/kpops/component_handlers/utils/exception.py index 5de7f7717..fe906190f 100644 --- a/kpops/component_handlers/utils/exception.py +++ b/kpops/component_handlers/utils/exception.py @@ -10,11 +10,11 @@ def __init__(self, response: httpx.Response) -> None: self.error_code = response.status_code self.error_msg = "Something went wrong!" try: - log.exception( - f"The request responded with the code {self.error_code}. Error body: {response.json()}", + log.error( + f"The request responded with the code {self.error_code}. Error body: {response.json()}" ) response.raise_for_status() except httpx.HTTPError as e: self.error_msg = str(e) - log.exception(f"More information: {self.error_msg}") + log.error(f"More information: {self.error_msg}") super().__init__() diff --git a/kpops/components/base_components/base_defaults_component.py b/kpops/components/base_components/base_defaults_component.py index d9100bd25..99dec42f2 100644 --- a/kpops/components/base_components/base_defaults_component.py +++ b/kpops/components/base_components/base_defaults_component.py @@ -77,15 +77,15 @@ def __init__(self, **kwargs) -> None: self._validate_custom(**kwargs) @cached_classproperty - def type(cls: type[Self]) -> str: # pyright: ignore[reportGeneralTypeIssues] - """Return calling component's type. + def type(cls: type[Self]) -> str: # pyright: ignore + """Return calling component's type :returns: Component class name in dash-case """ return to_dash(cls.__name__) def extend_with_defaults(self, **kwargs) -> dict: - """Merge parent components' defaults with own. + """Merge parent components' defaults with own :param kwargs: The init kwargs for pydantic :returns: Enriched kwargs with inheritted defaults @@ -105,13 +105,15 @@ def extend_with_defaults(self, **kwargs) -> dict: defaults = load_defaults( self.__class__, main_default_file_path, environment_default_file_path ) - return update_nested(kwargs, defaults) + kwargs = update_nested(kwargs, defaults) + return kwargs def _validate_custom(self, **kwargs) -> None: """Run custom validation on component. :param kwargs: The init kwargs for the component """ + pass def load_defaults( @@ -119,7 +121,7 @@ def load_defaults( defaults_file_path: Path, environment_defaults_file_path: Path | None = None, ) -> dict: - """Resolve component-specific defaults including environment defaults. + """Resolve component-specific defaults including environment defaults :param component_class: Component class :param defaults_file_path: Path to `defaults.yaml` @@ -151,7 +153,7 @@ def load_defaults( def defaults_from_yaml(path: Path, key: str) -> dict: - """Read component-specific settings from a defaults yaml file and return @default if not found. + """Read component-specific settings from a defaults yaml file and return @default if not found :param path: Path to defaults yaml file :param key: Component type @@ -163,10 +165,9 @@ def defaults_from_yaml(path: Path, key: str) -> dict: """ content = load_yaml_file(path, substitution=ENV) if not isinstance(content, dict): - msg = ( + raise TypeError( "Default files should be structured as map ([app type] -> [default config]" ) - raise TypeError(msg) value = content.get(key) if value is None: return {} @@ -177,7 +178,7 @@ def defaults_from_yaml(path: Path, key: str) -> dict: def get_defaults_file_paths(config: PipelineConfig) -> tuple[Path, Path]: - """Return the paths to the main and the environment defaults-files. + """Return the paths to the main and the environment defaults-files The files need not exist, this function will only check if the dir set in `config.defaults_path` exists and return paths to the defaults files diff --git a/kpops/components/base_components/kafka_app.py b/kpops/components/base_components/kafka_app.py index 7ffd3e530..ed6edfcf2 100644 --- a/kpops/components/base_components/kafka_app.py +++ b/kpops/components/base_components/kafka_app.py @@ -22,7 +22,7 @@ class KafkaStreamsConfig(BaseModel): - """Kafka Streams config. + """Kafka Streams config :param brokers: Brokers :param schema_registry_url: URL of the schema registry, defaults to None @@ -38,7 +38,7 @@ class Config(CamelCaseConfig, DescConfig): class KafkaAppConfig(KubernetesAppConfig): - """Settings specific to Kafka Apps. + """Settings specific to Kafka Apps :param streams: Kafka streams config :param name_override: Override name with this value, defaults to None @@ -82,8 +82,8 @@ class KafkaApp(KubernetesApp, ABC): @property def clean_up_helm_chart(self) -> str: - """Helm chart used to destroy and clean this component.""" - raise NotImplementedError + """Helm chart used to destroy and clean this component""" + raise NotImplementedError() @override async def deploy(self, dry_run: bool) -> None: @@ -104,7 +104,7 @@ def _run_clean_up_job( dry_run: bool, retain_clean_jobs: bool = False, ) -> None: - """Clean an app using the respective cleanup job. + """Clean an app using the respective cleanup job :param values: The value YAML for the chart :param dry_run: Dry run command @@ -133,7 +133,7 @@ def _run_clean_up_job( self.__uninstall_clean_up_job(clean_up_release_name, dry_run) def __uninstall_clean_up_job(self, release_name: str, dry_run: bool) -> None: - """Uninstall clean up job. + """Uninstall clean up job :param release_name: Name of the Helm release :param dry_run: Whether to do a dry run of the command @@ -147,7 +147,7 @@ def __install_clean_up_job( values: dict, dry_run: bool, ) -> str: - """Install clean up job. + """Install clean up job :param release_name: Name of the Helm release :param suffix: Suffix to add to the release name, e.g. "-clean" diff --git a/kpops/components/base_components/kafka_connector.py b/kpops/components/base_components/kafka_connector.py index 95ca80ab4..03edcdd8d 100644 --- a/kpops/components/base_components/kafka_connector.py +++ b/kpops/components/base_components/kafka_connector.py @@ -3,7 +3,7 @@ import logging from abc import ABC from functools import cached_property -from typing import TYPE_CHECKING, Any, NoReturn +from typing import Any, NoReturn from pydantic import Field, validator from typing_extensions import override @@ -25,18 +25,16 @@ KafkaConnectResetterValues, ) from kpops.components.base_components.base_defaults_component import deduplicate +from kpops.components.base_components.models.from_section import FromTopic from kpops.components.base_components.pipeline_component import PipelineComponent from kpops.utils.colorify import magentaify from kpops.utils.docstring import describe_attr -if TYPE_CHECKING: - from kpops.components.base_components.models.from_section import FromTopic - log = logging.getLogger("KafkaConnector") class KafkaConnector(PipelineComponent, ABC): - """Base class for all Kafka connectors. + """Base class for all Kafka connectors Should only be used to set defaults @@ -87,14 +85,13 @@ def connector_config_should_have_component_name( component_name = values["prefix"] + values["name"] connector_name: str | None = app.get("name") if connector_name is not None and connector_name != component_name: - msg = "Connector name should be the same as component name" - raise ValueError(msg) + raise ValueError("Connector name should be the same as component name") app["name"] = component_name return app @cached_property def helm(self) -> Helm: - """Helm object that contains component-specific config such as repo.""" + """Helm object that contains component-specific config such as repo""" helm_repo_config = self.repo_config helm = Helm(self.config.helm_config) helm.add_repo( @@ -108,7 +105,8 @@ def helm(self) -> Helm: def _resetter_release_name(self) -> str: suffix = "-clean" clean_up_release_name = self.full_name + suffix - return trim_release_name(clean_up_release_name, suffix) + trimmed_name = trim_release_name(clean_up_release_name, suffix) + return trimmed_name @property def _resetter_helm_chart(self) -> str: @@ -121,7 +119,7 @@ def dry_run_handler(self) -> DryRunHandler: @property def helm_flags(self) -> HelmFlags: - """Return shared flags for Helm commands.""" + """Return shared flags for Helm commands""" return HelmFlags( **self.repo_config.repo_auth_flags.dict(), version=self.version, @@ -130,7 +128,7 @@ def helm_flags(self) -> HelmFlags: @property def template_flags(self) -> HelmTemplateFlags: - """Return flags for Helm template command.""" + """Return flags for Helm template command""" return HelmTemplateFlags( **self.helm_flags.dict(), api_version=self.config.helm_config.api_version, @@ -173,7 +171,7 @@ def _run_connect_resetter( retain_clean_jobs: bool, **kwargs, ) -> None: - """Clean the connector from the cluster. + """Clean the connector from the cluster At first, it deletes the previous cleanup job (connector resetter) to make sure that there is no running clean job in the cluster. Then it releases a cleanup job. @@ -212,7 +210,7 @@ def __install_connect_resetter( dry_run: bool, **kwargs, ) -> str: - """Install connector resetter. + """Install connector resetter :param dry_run: Whether to dry run the command :return: The output of `helm upgrade --install` @@ -237,7 +235,7 @@ def _get_kafka_connect_resetter_values( self, **kwargs, ) -> dict: - """Get connector resetter helm chart values. + """Get connector resetter helm chart values :return: The Helm chart values of the connector resetter """ @@ -255,7 +253,7 @@ def _get_kafka_connect_resetter_values( } def __uninstall_connect_resetter(self, release_name: str, dry_run: bool) -> None: - """Uninstall connector resetter. + """Uninstall connector resetter :param release_name: Name of the release to be uninstalled :param dry_run: Whether to do a dry run of the command @@ -268,7 +266,7 @@ def __uninstall_connect_resetter(self, release_name: str, dry_run: bool) -> None class KafkaSourceConnector(KafkaConnector): - """Kafka source connector model. + """Kafka source connector model :param offset_topic: offset.storage.topic, more info: https://kafka.apache.org/documentation/#connect_running, @@ -284,8 +282,7 @@ class KafkaSourceConnector(KafkaConnector): @override def apply_from_inputs(self, name: str, topic: FromTopic) -> NoReturn: - msg = "Kafka source connector doesn't support FromSection" - raise NotImplementedError(msg) + raise NotImplementedError("Kafka source connector doesn't support FromSection") @override def template(self) -> None: @@ -311,7 +308,7 @@ async def clean(self, dry_run: bool) -> None: self.__run_kafka_connect_resetter(dry_run) def __run_kafka_connect_resetter(self, dry_run: bool) -> None: - """Run the connector resetter. + """Runs the connector resetter :param dry_run: Whether to do a dry run of the command """ @@ -323,7 +320,7 @@ def __run_kafka_connect_resetter(self, dry_run: bool) -> None: class KafkaSinkConnector(KafkaConnector): - """Kafka sink connector model.""" + """Kafka sink connector model""" _connector_type = KafkaConnectorType.SINK @@ -371,7 +368,7 @@ async def clean(self, dry_run: bool) -> None: def __run_kafka_connect_resetter( self, dry_run: bool, delete_consumer_group: bool ) -> None: - """Run the connector resetter. + """Runs the connector resetter :param dry_run: Whether to do a dry run of the command :param delete_consumer_group: Whether the consumer group should be deleted or not diff --git a/kpops/components/base_components/kubernetes_app.py b/kpops/components/base_components/kubernetes_app.py index c4c7d04c5..8d885afda 100644 --- a/kpops/components/base_components/kubernetes_app.py +++ b/kpops/components/base_components/kubernetes_app.py @@ -30,7 +30,7 @@ class KubernetesAppConfig(BaseModel): - """Settings specific to Kubernetes Apps.""" + """Settings specific to Kubernetes Apps""" class Config(CamelCaseConfig, DescConfig): extra = Extra.allow @@ -68,7 +68,7 @@ class KubernetesApp(PipelineComponent): @cached_property def helm(self) -> Helm: - """Helm object that contains component-specific config such as repo.""" + """Helm object that contains component-specific config such as repo""" helm = Helm(self.config.helm_config) if self.repo_config is not None: helm.add_repo( @@ -80,7 +80,7 @@ def helm(self) -> Helm: @cached_property def helm_diff(self) -> HelmDiff: - """Helm diff object of last and current release of this component.""" + """Helm diff object of last and current release of this component""" return HelmDiff(self.config.helm_diff_config) @cached_property @@ -95,15 +95,14 @@ def helm_release_name(self) -> str: @property def helm_chart(self) -> str: - """Return component's Helm chart.""" - msg = ( + """Return component's Helm chart""" + raise NotImplementedError( f"Please implement the helm_chart property of the {self.__module__} module." ) - raise NotImplementedError(msg) @property def helm_flags(self) -> HelmFlags: - """Return shared flags for Helm commands.""" + """Return shared flags for Helm commands""" auth_flags = self.repo_config.repo_auth_flags.dict() if self.repo_config else {} return HelmFlags( **auth_flags, @@ -113,7 +112,7 @@ def helm_flags(self) -> HelmFlags: @property def template_flags(self) -> HelmTemplateFlags: - """Return flags for Helm template command.""" + """Return flags for Helm template command""" return HelmTemplateFlags( **self.helm_flags.dict(), api_version=self.config.helm_config.api_version, @@ -132,7 +131,7 @@ def template(self) -> None: @property def deploy_flags(self) -> HelmUpgradeInstallFlags: - """Return flags for Helm upgrade install command.""" + """Return flags for Helm upgrade install command""" return HelmUpgradeInstallFlags(**self.helm_flags.dict()) @override @@ -160,14 +159,14 @@ async def destroy(self, dry_run: bool) -> None: log.info(magentaify(stdout)) def to_helm_values(self) -> dict: - """Generate a dictionary of values readable by Helm from `self.app`. + """Generate a dictionary of values readable by Helm from `self.app` :returns: Thte values to be used by Helm """ return self.app.dict(by_alias=True, exclude_none=True, exclude_defaults=True) def print_helm_diff(self, stdout: str) -> None: - """Print the diff of the last and current release of this component. + """Print the diff of the last and current release of this component :param stdout: The output of a Helm command that installs or upgrades the release """ @@ -188,14 +187,13 @@ def _validate_custom(self, **kwargs) -> None: @staticmethod def validate_kubernetes_name(name: str) -> None: - """Check if a name is valid for a Kubernetes resource. + """Check if a name is valid for a Kubernetes resource :param name: Name that is to be used for the resource :raises ValueError: The component name {name} is invalid for Kubernetes. """ if not bool(KUBERNETES_NAME_CHECK_PATTERN.match(name)): - msg = f"The component name {name} is invalid for Kubernetes." - raise ValueError(msg) + raise ValueError(f"The component name {name} is invalid for Kubernetes.") @override def dict(self, *, exclude=None, **kwargs) -> dict[str, Any]: diff --git a/kpops/components/base_components/models/from_section.py b/kpops/components/base_components/models/from_section.py index 153133639..a3188a17b 100644 --- a/kpops/components/base_components/models/from_section.py +++ b/kpops/components/base_components/models/from_section.py @@ -9,7 +9,7 @@ class InputTopicTypes(str, Enum): - """Input topic types. + """Input topic types INPUT (input topic), PATTERN (extra-topic-pattern or input-topic-pattern) """ @@ -19,7 +19,7 @@ class InputTopicTypes(str, Enum): class FromTopic(BaseModel): - """Input topic. + """Input topic :param type: Topic type, defaults to None :param role: Custom identifier belonging to a topic; @@ -37,10 +37,9 @@ class Config(DescConfig): @root_validator def extra_topic_role(cls, values: dict[str, Any]) -> dict[str, Any]: - """Ensure that cls.role is used correctly, assign type if needed.""" + """Ensure that cls.role is used correctly, assign type if needed""" if values["type"] == InputTopicTypes.INPUT and values["role"]: - msg = "Define role only if `type` is `pattern` or `None`" - raise ValueError(msg) + raise ValueError("Define role only if `type` is `pattern` or `None`") return values @@ -48,7 +47,7 @@ def extra_topic_role(cls, values: dict[str, Any]) -> dict[str, Any]: class FromSection(BaseModel): - """Holds multiple input topics. + """Holds multiple input topics :param topics: Input topics :param components: Components to read from diff --git a/kpops/components/base_components/models/to_section.py b/kpops/components/base_components/models/to_section.py index 03f1d7141..cbad0987a 100644 --- a/kpops/components/base_components/models/to_section.py +++ b/kpops/components/base_components/models/to_section.py @@ -9,7 +9,7 @@ class OutputTopicTypes(str, Enum): - """Types of output topic. + """Types of output topic OUTPUT (output topic), ERROR (error topic) """ @@ -19,7 +19,7 @@ class OutputTopicTypes(str, Enum): class TopicConfig(BaseModel): - """Configure an output topic. + """Configure an output topic :param type: Topic type :param key_schema: Key schema class name @@ -65,15 +65,14 @@ class Config(DescConfig): @root_validator def extra_topic_role(cls, values: dict[str, Any]) -> dict[str, Any]: - """Ensure that cls.role is used correctly, assign type if needed.""" + """Ensure that cls.role is used correctly, assign type if needed""" if values["type"] and values["role"]: - msg = "Define `role` only if `type` is undefined" - raise ValueError(msg) + raise ValueError("Define `role` only if `type` is undefined") return values class ToSection(BaseModel): - """Holds multiple output topics. + """Holds multiple output topics :param topics: Output topics :param models: Data models diff --git a/kpops/components/streams_bootstrap/producer/model.py b/kpops/components/streams_bootstrap/producer/model.py index 8af1a68c6..3c4ae6e46 100644 --- a/kpops/components/streams_bootstrap/producer/model.py +++ b/kpops/components/streams_bootstrap/producer/model.py @@ -8,7 +8,7 @@ class ProducerStreamsConfig(KafkaStreamsConfig): - """Kafka Streams settings specific to Producer. + """Kafka Streams settings specific to Producer :param extra_output_topics: Extra output topics :param output_topic: Output topic, defaults to None @@ -23,7 +23,7 @@ class ProducerStreamsConfig(KafkaStreamsConfig): class ProducerValues(KafkaAppConfig): - """Settings specific to producers. + """Settings specific to producers :param streams: Kafka Streams settings """ diff --git a/kpops/components/streams_bootstrap/producer/producer_app.py b/kpops/components/streams_bootstrap/producer/producer_app.py index 0d3dfde90..e94318286 100644 --- a/kpops/components/streams_bootstrap/producer/producer_app.py +++ b/kpops/components/streams_bootstrap/producer/producer_app.py @@ -1,4 +1,4 @@ -# from __future__ import annotations +from __future__ import annotations from pydantic import Field from typing_extensions import override @@ -14,7 +14,7 @@ class ProducerApp(KafkaApp): - """Producer component. + """Producer component This producer holds configuration to use as values for the streams bootstrap producer helm chart. @@ -40,8 +40,7 @@ class ProducerApp(KafkaApp): def apply_to_outputs(self, name: str, topic: TopicConfig) -> None: match topic.type: case OutputTopicTypes.ERROR: - msg = "Producer apps do not support error topics" - raise ValueError(msg) + raise ValueError("Producer apps do not support error topics") case _: super().apply_to_outputs(name, topic) diff --git a/kpops/components/streams_bootstrap/streams/model.py b/kpops/components/streams_bootstrap/streams/model.py index ca2db77ae..aabbe8237 100644 --- a/kpops/components/streams_bootstrap/streams/model.py +++ b/kpops/components/streams_bootstrap/streams/model.py @@ -1,5 +1,4 @@ -from collections.abc import Mapping, Set -from typing import Any +from typing import AbstractSet, Any, Mapping from pydantic import BaseConfig, BaseModel, Extra, Field from typing_extensions import override @@ -14,7 +13,7 @@ class StreamsConfig(KafkaStreamsConfig): - """Streams Bootstrap streams section. + """Streams Bootstrap streams section :param input_topics: Input topics, defaults to [] :param input_pattern: Input pattern, defaults to None @@ -76,14 +75,14 @@ def add_extra_input_topics(self, role: str, topics: list[str]) -> None: def dict( self, *, - include: None | Set[int | str] | Mapping[int | str, Any] = None, - exclude: None | Set[int | str] | Mapping[int | str, Any] = None, + include: None | AbstractSet[int | str] | Mapping[int | str, Any] = None, + exclude: None | AbstractSet[int | str] | Mapping[int | str, Any] = None, by_alias: bool = False, skip_defaults: bool | None = None, exclude_unset: bool = False, **kwargs, ) -> dict: - """Generate a dictionary representation of the model. + """Generate a dictionary representation of the model Optionally, specify which fields to include or exclude. @@ -106,7 +105,7 @@ def dict( class StreamsAppAutoScaling(BaseModel): - """Kubernetes Event-driven Autoscaling config. + """Kubernetes Event-driven Autoscaling config :param enabled: Whether to enable auto-scaling using KEDA., defaults to False :param consumer_group: Name of the consumer group used for checking the diff --git a/kpops/components/streams_bootstrap/streams/streams_app.py b/kpops/components/streams_bootstrap/streams/streams_app.py index e37bddc44..ba0c1ee39 100644 --- a/kpops/components/streams_bootstrap/streams/streams_app.py +++ b/kpops/components/streams_bootstrap/streams/streams_app.py @@ -1,3 +1,5 @@ +from __future__ import annotations + from pydantic import Field from typing_extensions import override @@ -8,7 +10,7 @@ class StreamsApp(KafkaApp): - """StreamsApp component that configures a streams bootstrap app. + """StreamsApp component that configures a streams bootstrap app :param app: Application-specific settings """ @@ -81,7 +83,7 @@ async def clean(self, dry_run: bool) -> None: self.__run_streams_clean_up_job(dry_run, delete_output=True) def __run_streams_clean_up_job(self, dry_run: bool, delete_output: bool) -> None: - """Run clean job for this Streams app. + """Run clean job for this Streams app :param dry_run: Whether to do a dry run of the command :param delete_output: Whether to delete the output of the app that is being cleaned diff --git a/kpops/utils/dict_differ.py b/kpops/utils/dict_differ.py index 934924e21..2cdaa95b0 100644 --- a/kpops/utils/dict_differ.py +++ b/kpops/utils/dict_differ.py @@ -3,15 +3,12 @@ from dataclasses import dataclass from difflib import Differ from enum import Enum -from typing import TYPE_CHECKING, Generic, TypeVar +from typing import Generic, Iterable, Iterator, Sequence, TypeVar import typer import yaml from dictdiffer import diff, patch -if TYPE_CHECKING: - from collections.abc import Iterable, Iterator, Sequence - differ = Differ() @@ -42,8 +39,7 @@ def factory(type: DiffType, change: T | tuple[T, T]) -> Change: return Change(change, None) case DiffType.CHANGE if isinstance(change, tuple): return Change(*change) - msg = f"{type} is not part of {DiffType}" - raise ValueError(msg) + raise ValueError(f"{type} is not part of {DiffType}") @dataclass @@ -57,9 +53,9 @@ def from_dicts( d1: dict, d2: dict, ignore: set[str] | None = None ) -> Iterator[Diff]: for diff_type, keys, changes in diff(d1, d2, ignore=ignore): - if not isinstance(changes_tmp := changes, list): - changes_tmp = [("", changes)] - for key, change in changes_tmp: + if not isinstance(changes, list): + changes = [("", changes)] + for key, change in changes: yield Diff( DiffType.from_str(diff_type), Diff.__find_changed_key(keys, key), @@ -68,7 +64,9 @@ def from_dicts( @staticmethod def __find_changed_key(key_1: list[str] | str, key_2: str = "") -> str: - """Generate a string that points to the changed key in the dictionary.""" + """ + Generates a string that points to the changed key in the dictionary. + """ if isinstance(key_1, list) and len(key_1) > 1: return f"{key_1[0]}[{key_1[1]}]" if not key_1: diff --git a/kpops/utils/dict_ops.py b/kpops/utils/dict_ops.py index 14cc849e3..64e88a89b 100644 --- a/kpops/utils/dict_ops.py +++ b/kpops/utils/dict_ops.py @@ -1,9 +1,8 @@ -from collections.abc import Mapping -from typing import Any +from typing import Any, Mapping def update_nested_pair(original_dict: dict, other_dict: Mapping) -> dict: - """Nested update for 2 dictionaries. + """Nested update for 2 dictionaries Adds all new fields in ``other_dict`` to ``original_dict``. Does not update existing fields. @@ -20,8 +19,9 @@ def update_nested_pair(original_dict: dict, other_dict: Mapping) -> dict: nested_val = original_dict.get(key, {}) if isinstance(nested_val, dict): original_dict[key] = update_nested_pair(nested_val, value) - elif key not in original_dict: - original_dict[key] = value + else: + if key not in original_dict: + original_dict[key] = value return original_dict @@ -48,7 +48,7 @@ def update_nested(*argv: dict) -> dict: def flatten_mapping( nested_mapping: Mapping[str, Any], prefix: str | None = None, separator: str = "_" ) -> dict[str, Any]: - """Flattens a Mapping. + """Flattens a Mapping :param nested_mapping: Nested mapping that is to be flattened :param prefix: Prefix that will be applied to all top-level keys in the output., defaults to None @@ -56,13 +56,11 @@ def flatten_mapping( :returns: "Flattened" mapping in the form of dict """ if not isinstance(nested_mapping, Mapping): - msg = "Argument nested_mapping is not a Mapping" - raise TypeError(msg) + raise TypeError("Argument nested_mapping is not a Mapping") top: dict[str, Any] = {} for key, value in nested_mapping.items(): if not isinstance(key, str): - msg = f"Argument nested_mapping contains a non-str key: {key}" - raise TypeError(msg) + raise TypeError(f"Argument nested_mapping contains a non-str key: {key}") if prefix: key = prefix + separator + key if isinstance(value, Mapping): @@ -78,7 +76,7 @@ def generate_substitution( prefix: str | None = None, existing_substitution: dict | None = None, ) -> dict: - """Generate a complete substitution dict from a given dict. + """Generate a complete substitution dict from a given dict Finds all attributes that belong to a model and expands them to create a dict containing each variable name and value to substitute with. diff --git a/kpops/utils/docstring.py b/kpops/utils/docstring.py index d5ca287d3..fc6f4c61d 100644 --- a/kpops/utils/docstring.py +++ b/kpops/utils/docstring.py @@ -4,7 +4,7 @@ def describe_attr(name: str, docstr: str | None) -> str: - """Read attribute description from class docstring. + """Read attribute description from class docstring **Works only with reStructuredText docstrings.** @@ -19,7 +19,7 @@ def describe_attr(name: str, docstr: str | None) -> str: def describe_object(docstr: str | None) -> str: - """Return description from an object's docstring. + """Return description from an object's docstring Excludes parameters and return definitions @@ -44,7 +44,7 @@ def describe_object(docstr: str | None) -> str: def _trim_description_end(desc: str) -> str: - """Remove the unwanted text that comes after a description in a docstring. + """Remove the unwanted text that comes after a description in a docstring Also removes all whitespaces and newlines and replaces them with a single space. diff --git a/kpops/utils/environment.py b/kpops/utils/environment.py index 0ed7ae920..c46f83611 100644 --- a/kpops/utils/environment.py +++ b/kpops/utils/environment.py @@ -1,7 +1,7 @@ import os import platform from collections import UserDict -from collections.abc import Callable +from typing import Callable class Environment(UserDict): diff --git a/kpops/utils/gen_schema.py b/kpops/utils/gen_schema.py index 7cad9422d..470a1412d 100644 --- a/kpops/utils/gen_schema.py +++ b/kpops/utils/gen_schema.py @@ -1,9 +1,8 @@ import inspect import logging from abc import ABC -from collections.abc import Sequence from enum import Enum -from typing import Annotated, Any, Literal, Union +from typing import Annotated, Any, Literal, Sequence, Union from pydantic import BaseConfig, Field, schema, schema_json_of from pydantic.fields import FieldInfo, ModelField @@ -26,8 +25,7 @@ class SchemaScope(str, Enum): # adapted from https://github.com/tiangolo/fastapi/issues/1378#issuecomment-764966955 def field_schema(field: ModelField, **kwargs: Any) -> Any: if field.field_info.extra.get("hidden_from_schema"): - msg = f"{field.name} field is being hidden" - raise SkipField(msg) + raise SkipField(f"{field.name} field is being hidden") else: return original_field_schema(field, **kwargs) @@ -40,7 +38,8 @@ def field_schema(field: ModelField, **kwargs: Any) -> Any: def _is_valid_component( defined_component_types: set[str], component: type[PipelineComponent] ) -> bool: - """Check whether a PipelineComponent subclass has a valid definition for the schema generation. + """ + Check whether a PipelineComponent subclass has a valid definition for the schema generation. :param defined_component_types: types defined so far :param component: component type to be validated @@ -59,7 +58,7 @@ def _is_valid_component( def _add_components( components_module: str, components: tuple[type[PipelineComponent]] | None = None ) -> tuple[type[PipelineComponent]]: - """Add components to a components tuple. + """Add components to a components tuple If an empty tuple is provided or it is not provided at all, the components types from the given module are 'tupled' @@ -70,7 +69,7 @@ def _add_components( :return: Extended tuple """ if components is None: - components = tuple() # noqa: C408 + components = tuple() # Set of existing types, against which to check the new ones defined_component_types = {component.type for component in components} custom_components = ( @@ -96,15 +95,14 @@ def gen_pipeline_schema( log.warning("No components are provided, no schema is generated.") return # Add stock components if enabled - components: tuple[type[PipelineComponent]] = tuple() # noqa: C408 + components: tuple[type[PipelineComponent]] = tuple() if include_stock_components: components = _add_components("kpops.components") # Add custom components if provided if components_module: components = _add_components(components_module, components) if not components: - msg = "No valid components found." - raise RuntimeError(msg) + raise RuntimeError("No valid components found.") # Create a type union that will hold the union of all component types PipelineComponents = Union[components] # type: ignore[valid-type] @@ -112,7 +110,7 @@ def gen_pipeline_schema( for component in components: component.__fields__["type"] = ModelField( name="type", - type_=Literal[component.type], # type: ignore[reportGeneralTypeIssues] + type_=Literal[component.type], # type: ignore required=False, default=component.type, final=True, @@ -139,7 +137,7 @@ def gen_pipeline_schema( def gen_config_schema() -> None: - """Generate a json schema from the model of pipeline config.""" + """Generate a json schema from the model of pipeline config""" schema = schema_json_of( PipelineConfig, title="KPOps config schema", indent=4, sort_keys=True ) diff --git a/kpops/utils/yaml_loading.py b/kpops/utils/yaml_loading.py index fb810c193..cb9536200 100644 --- a/kpops/utils/yaml_loading.py +++ b/kpops/utils/yaml_loading.py @@ -20,7 +20,7 @@ def generate_hashkey( def load_yaml_file( file_path: Path, *, substitution: Mapping[str, Any] | None = None ) -> dict | list[dict]: - with file_path.open() as yaml_file: + with open(file_path) as yaml_file: return yaml.load(substitute(yaml_file.read(), substitution), Loader=yaml.Loader) @@ -70,6 +70,7 @@ def substitute_nested(input: str, **kwargs) -> str: steps.add(new_str) old_str, new_str = new_str, substitute(new_str, kwargs) if new_str != old_str: - msg = "An infinite loop condition detected. Check substitution variables." - raise ValueError(msg) + raise ValueError( + "An infinite loop condition detected. Check substitution variables." + ) return old_str diff --git a/pyproject.toml b/pyproject.toml index 5dddd436e..8d2435173 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -47,8 +47,9 @@ pytest-mock = "^3.10.0" pytest-timeout = "^2.1.0" snapshottest = "^0.6.0" pre-commit = "^2.19.0" -ruff = "^0.0.292" +flake8 = "^4.0.1" black = "^23.7.0" +isort = "^5.12.0" typer-cli = "^0.0.13" pyright = "^1.1.314" pytest-rerunfailures = "^11.1.2" @@ -71,101 +72,6 @@ mike = "^1.1.2" [tool.poetry_bumpversion.file."kpops/__init__.py"] -[tool.ruff] -ignore = [ - # "E203", # whitespace before ':' -- Not PEP8 compliant, black won't correct it, add when out of nursery - "E501", # Line too long -- Clashes with `black` - "D1", # Missing docstring for {} -- Inconvenient to enforce -# The following "D" rules do not correspond to our coding style. We use the pep257 convention, but -# "D212" should not be ignored. In ruff (0.0.291) we cannot select a rule that is excluded by specifying -# a convention, hence our only option is to manually replicate it. - "D203", # 1 blank line required before class docstring - "D213", # Multi-line docstring summary should start at the second line - "D214", # Section is over-indented ("{name}") - "D215", # Section underline is over-indented ("{name}") - "D404", # First word of the docstring should not be "This" - "D405", # Section name should be properly capitalized ("{name}") - "D406", # Section name should end with a newline ("{name}") - "D407", # Missing dashed underline after section ("{name}") - "D408", # Section underline should be in the line following the section's name ("{name}") - "D409", # Section underline should match the length of its name ("{name}") - "D410", # Missing blank line after section ("{name}") - "D411", # Missing blank line before section ("{name}") - "D413", # Missing blank line after last section ("{name}") - "D415", # First line should end with a period, question mark, or exclamation point - "D416", # Section name should end with a colon ("{name}") - "D417", # Missing argument description in the docstring for {definition}: {name} - "B009", # Do not call getattr with a constant attribute value. -- Not always applicable - "B010", # Do not call setattr with a constant attribute value. -- Not always applicable - "RUF012", # type class attrs with `ClassVar` -- Too strict/trigger-happy - "UP007", # Use X | Y for type annotations -- `typer` doesn't support it - "COM812", # Checks for the absence of trailing commas -- leads to undesirable behavior from formatters - "PIE804", # Unnecessary `dict` kwargs -- Inconvenient to enforce - "RET505", # Unnecessary {branch} after return statement -- Lots of false positives - "RET506", # Unnecessary {branch} after raise statement -- Lots of false positives - "RET507", # Unnecessary {branch} after continue statement -- Lots of false positives - "RET508", # Unnecessary {branch} after break statement -- Lots of false positives - "PLR09", # upper bound on number of arguments, functions, etc. -- Inconvenient to enforce - "PLR2004", # Magic value used in comparison, consider replacing {value} with a constant variable -- Inconvenient to enforce - "PLW2901", # `for` loop variable `{var}` overwritten by assignment target -- Inconvenient to enforce - "TRY002", # Create your own exception -- Inconvenient to enforce - "TRY003", # Avoid specifying long messages outside the exception class -- Inconvenient to enforce -] -select = [ - "F", # Pyflakes - "E", # pycodestyle Errors - "W", # pycodestyle Warnings - "C90", # mccabe - "I", # isort - "D", # pydocstyle - "UP", # pyupgrade - "B", # flake8-bugbear - "INP", # flake8-no-pep420 - "RUF", # Ruff-specific rules - "YTT", # flake8-2020 - "ASYNC", # flake8-async - "BLE", # flake8-blind-except - "COM", # flake8-commas - "C4", # flake8-comprehensions - "T10", # flake8-debugger - "EM", # flake8-errmsg - "FA", # flake8-future-annotations - "ISC", # flake8-implicit-str-concat - "ICN", # flake8-import-conventions - "INP", # flake8-no-pep420 - "PIE", # flake8-pie - "PT", # flake8-pytest-style - "Q", # flake8-quotes - "RSE", # flake8-raise - "RET", # flake8-return - "SLOT", # flake8-slots - "SIM", # flake8-simplify - "TCH", # flake8-type-checking, configure correctly and add - "PTH", # flake8-use-pathlib - "PGH", # pygrep-hooks - "PL", # Pylint - "TRY", # tryceratops - # "FURB", # refurb, add when out of nursery - # "LOG", # flake8-logging, add when out of nursery -] -output-format = "grouped" -show-fixes = true -task-tags = ["TODO", "HACK", "FIXME", "XXX"] -target-version = "py310" -exclude = ["tests/*snapshots/*"] - -[tool.ruff.extend-per-file-ignores] -"tests/*/__init__.py" = ["F401"] - -[tool.ruff.isort] -split-on-trailing-comma = false - -[tool.ruff.flake8-bugbear] -extend-immutable-calls = ["typer.Argument"] - -[tool.ruff.flake8-type-checking] -runtime-evaluated-base-classes = ["pydantic.BaseModel"] - [build-system] requires = ["poetry-core>=1.0.0"] build-backend = "poetry.core.masonry.api" diff --git a/setup.cfg b/setup.cfg new file mode 100644 index 000000000..89429d3e0 --- /dev/null +++ b/setup.cfg @@ -0,0 +1,19 @@ +[flake8] +exclude = + .git, + __pycache__ +max-complexity = 10 +# black docs regarding flake8: https://black.readthedocs.io/en/stable/guides/using_black_with_other_tools.html#flake8 +# black enforces an equal amount of whitespace around slice operators. It is not PEP8 compliant. +# black and flake8 also disagree on line length +extend-ignore = + # E203: Whitespace before ':' + E203, + # E501: Line too long + E501, +per-file-ignores = + # F401: unused imports + tests/*/__init__.py: F401 + +[isort] +profile = black diff --git a/tests/cli/snapshots/snap_test_schema_generation.py b/tests/cli/snapshots/snap_test_schema_generation.py index 2dd92b512..2a19e65c1 100644 --- a/tests/cli/snapshots/snap_test_schema_generation.py +++ b/tests/cli/snapshots/snap_test_schema_generation.py @@ -58,7 +58,7 @@ }, "FromSection": { "additionalProperties": false, - "description": "Holds multiple input topics.", + "description": "Holds multiple input topics", "properties": { "components": { "additionalProperties": { @@ -84,7 +84,7 @@ }, "FromTopic": { "additionalProperties": false, - "description": "Input topic.", + "description": "Input topic", "properties": { "role": { "description": "Custom identifier belonging to a topic; define only if `type` is `pattern` or `None`", @@ -104,7 +104,7 @@ "type": "object" }, "InputTopicTypes": { - "description": "Input topic types.\\n\\nINPUT (input topic), PATTERN (extra-topic-pattern or input-topic-pattern)", + "description": "Input topic types\\n\\nINPUT (input topic), PATTERN (extra-topic-pattern or input-topic-pattern)", "enum": [ "input", "pattern" @@ -113,7 +113,7 @@ "type": "string" }, "OutputTopicTypes": { - "description": "Types of output topic.\\n\\nOUTPUT (output topic), ERROR (error topic)", + "description": "Types of output topic\\n\\nOUTPUT (output topic), ERROR (error topic)", "enum": [ "output", "error" @@ -216,7 +216,7 @@ "type": "object" }, "SubPipelineComponentCorrectDocstr": { - "description": "Newline before title is removed.\\nSummarry is correctly imported. All whitespaces are removed and replaced with a single space. The description extraction terminates at the correct place, deletes 1 trailing coma", + "description": "Newline before title is removed\\nSummarry is correctly imported. All whitespaces are removed and replaced with a single space. The description extraction terminates at the correct place, deletes 1 trailing coma", "properties": { "example_attr": { "description": "Parameter description looks correct and it is not included in the class description, terminates here", @@ -254,7 +254,7 @@ }, "type": { "default": "sub-pipeline-component-correct-docstr", - "description": "Newline before title is removed.\\nSummarry is correctly imported. All whitespaces are removed and replaced with a single space. The description extraction terminates at the correct place, deletes 1 trailing coma", + "description": "Newline before title is removed\\nSummarry is correctly imported. All whitespaces are removed and replaced with a single space. The description extraction terminates at the correct place, deletes 1 trailing coma", "enum": [ "sub-pipeline-component-correct-docstr" ], @@ -317,7 +317,7 @@ "type": "object" }, "ToSection": { - "description": "Holds multiple output topics.", + "description": "Holds multiple output topics", "properties": { "models": { "additionalProperties": { @@ -343,7 +343,7 @@ }, "TopicConfig": { "additionalProperties": false, - "description": "Configure an output topic.", + "description": "Configure an output topic", "properties": { "configs": { "additionalProperties": { diff --git a/tests/cli/test_schema_generation.py b/tests/cli/test_schema_generation.py index cbb855d14..6c651dfa4 100644 --- a/tests/cli/test_schema_generation.py +++ b/tests/cli/test_schema_generation.py @@ -3,19 +3,16 @@ import logging from abc import ABC, abstractmethod from pathlib import Path -from typing import TYPE_CHECKING import pytest from pydantic import Field +from snapshottest.module import SnapshotTest from typer.testing import CliRunner +import tests.cli.resources.empty_module as empty_module from kpops.cli.main import app from kpops.components.base_components import PipelineComponent from kpops.utils.docstring import describe_attr -from tests.cli.resources import empty_module - -if TYPE_CHECKING: - from snapshottest.module import SnapshotTest RESOURCE_PATH = Path(__file__).parent / "resources" @@ -57,7 +54,8 @@ class SubPipelineComponentCorrect(SubPipelineComponent): # Correctly defined, docstr test class SubPipelineComponentCorrectDocstr(SubPipelineComponent): - """Newline before title is removed. + """ + Newline before title is removed Summarry is correctly imported. All @@ -110,7 +108,7 @@ def test_gen_pipeline_schema_no_modules(self, caplog: pytest.LogCaptureFixture): def test_gen_pipeline_schema_no_components(self): with pytest.raises(RuntimeError, match="^No valid components found.$"): - runner.invoke( + result = runner.invoke( app, [ "schema", @@ -120,6 +118,7 @@ def test_gen_pipeline_schema_no_components(self): ], catch_exceptions=False, ) + assert result.exit_code == 1 def test_gen_pipeline_schema_only_stock_module(self): result = runner.invoke( diff --git a/tests/compiler/test_pipeline_name.py b/tests/compiler/test_pipeline_name.py index f0a1b1b1e..7a07c1a12 100644 --- a/tests/compiler/test_pipeline_name.py +++ b/tests/compiler/test_pipeline_name.py @@ -8,51 +8,49 @@ DEFAULTS_PATH = Path(__file__).parent / "resources" PIPELINE_PATH = Path("./some/random/path/for/testing/pipeline.yaml") -DEFAULT_BASE_DIR = Path() +DEFAULT_BASE_DIR = Path(".") def test_should_set_pipeline_name_with_default_base_dir(): Pipeline.set_pipeline_name_env_vars(DEFAULT_BASE_DIR, PIPELINE_PATH) - assert ENV["pipeline_name"] == "some-random-path-for-testing" - assert ENV["pipeline_name_0"] == "some" - assert ENV["pipeline_name_1"] == "random" - assert ENV["pipeline_name_2"] == "path" - assert ENV["pipeline_name_3"] == "for" - assert ENV["pipeline_name_4"] == "testing" + assert "some-random-path-for-testing" == ENV["pipeline_name"] + assert "some" == ENV["pipeline_name_0"] + assert "random" == ENV["pipeline_name_1"] + assert "path" == ENV["pipeline_name_2"] + assert "for" == ENV["pipeline_name_3"] + assert "testing" == ENV["pipeline_name_4"] def test_should_set_pipeline_name_with_specific_relative_base_dir(): Pipeline.set_pipeline_name_env_vars(Path("./some/random/path"), PIPELINE_PATH) - assert ENV["pipeline_name"] == "for-testing" - assert ENV["pipeline_name_0"] == "for" - assert ENV["pipeline_name_1"] == "testing" + assert "for-testing" == ENV["pipeline_name"] + assert "for" == ENV["pipeline_name_0"] + assert "testing" == ENV["pipeline_name_1"] def test_should_set_pipeline_name_with_specific_absolute_base_dir(): Pipeline.set_pipeline_name_env_vars(Path("some/random/path"), PIPELINE_PATH) - assert ENV["pipeline_name"] == "for-testing" - assert ENV["pipeline_name_0"] == "for" - assert ENV["pipeline_name_1"] == "testing" + assert "for-testing" == ENV["pipeline_name"] + assert "for" == ENV["pipeline_name_0"] + assert "testing" == ENV["pipeline_name_1"] def test_should_set_pipeline_name_with_absolute_base_dir(): Pipeline.set_pipeline_name_env_vars(Path.cwd(), PIPELINE_PATH) - assert ENV["pipeline_name"] == "some-random-path-for-testing" - assert ENV["pipeline_name_0"] == "some" - assert ENV["pipeline_name_1"] == "random" - assert ENV["pipeline_name_2"] == "path" - assert ENV["pipeline_name_3"] == "for" - assert ENV["pipeline_name_4"] == "testing" + assert "some-random-path-for-testing" == ENV["pipeline_name"] + assert "some" == ENV["pipeline_name_0"] + assert "random" == ENV["pipeline_name_1"] + assert "path" == ENV["pipeline_name_2"] + assert "for" == ENV["pipeline_name_3"] + assert "testing" == ENV["pipeline_name_4"] def test_should_not_set_pipeline_name_with_the_same_base_dir(): - with pytest.raises( - ValueError, match="The pipeline-base-dir should not equal the pipeline-path" - ): + with pytest.raises(ValueError): Pipeline.set_pipeline_name_env_vars(PIPELINE_PATH, PIPELINE_PATH) diff --git a/tests/component_handlers/helm_wrapper/test_dry_run_handler.py b/tests/component_handlers/helm_wrapper/test_dry_run_handler.py index bad4f2aa8..20c02f50d 100644 --- a/tests/component_handlers/helm_wrapper/test_dry_run_handler.py +++ b/tests/component_handlers/helm_wrapper/test_dry_run_handler.py @@ -12,13 +12,13 @@ class TestDryRunHandler: - @pytest.fixture() + @pytest.fixture def helm_mock(self, mocker: MockerFixture) -> MagicMock: return mocker.patch( "kpops.component_handlers.helm_wrapper.dry_run_handler.Helm" ).return_value - @pytest.fixture() + @pytest.fixture def helm_diff_mock(self, mocker: MockerFixture) -> MagicMock: return mocker.patch( "kpops.component_handlers.helm_wrapper.dry_run_handler.HelmDiff" diff --git a/tests/component_handlers/helm_wrapper/test_helm_wrapper.py b/tests/component_handlers/helm_wrapper/test_helm_wrapper.py index ce6fae709..de23dca8e 100644 --- a/tests/component_handlers/helm_wrapper/test_helm_wrapper.py +++ b/tests/component_handlers/helm_wrapper/test_helm_wrapper.py @@ -29,15 +29,15 @@ def temp_file_mock(self, mocker: MockerFixture) -> MagicMock: temp_file_mock.return_value.__enter__.return_value.name = "values.yaml" return temp_file_mock - @pytest.fixture() + @pytest.fixture def run_command(self, mocker: MockerFixture) -> MagicMock: return mocker.patch.object(Helm, "_Helm__execute") - @pytest.fixture() + @pytest.fixture def log_warning_mock(self, mocker: MockerFixture) -> MagicMock: return mocker.patch("kpops.component_handlers.helm_wrapper.helm.log.warning") - @pytest.fixture() + @pytest.fixture def mock_get_version(self, mocker: MockerFixture) -> MagicMock: mock_get_version = mocker.patch.object(Helm, "get_version") mock_get_version.return_value = Version(major=3, minor=12, patch=0) @@ -337,7 +337,8 @@ def test_raise_parse_error_when_helm_content_is_invalid(self): """ ) with pytest.raises(ParseError, match="Not a valid Helm template source"): - list(Helm.load_manifest(stdout)) + helm_template = list(Helm.load_manifest(stdout)) + assert len(helm_template) == 0 def test_load_manifest(self): stdout = dedent( @@ -497,7 +498,7 @@ def test_should_call_run_command_method_when_helm_template_without_optional_args ) @pytest.mark.parametrize( - ("raw_version", "expected_version"), + "raw_version, expected_version", [ ("v3.12.0+gc9f554d", Version(3, 12, 0)), ("v3.12.0", Version(3, 12, 0)), diff --git a/tests/components/test_base_defaults_component.py b/tests/components/test_base_defaults_component.py index d066d431b..7b25e5f74 100644 --- a/tests/components/test_base_defaults_component.py +++ b/tests/components/test_base_defaults_component.py @@ -37,7 +37,7 @@ class EnvVarTest(BaseDefaultsComponent): name: str | None = None -@pytest.fixture() +@pytest.fixture def config() -> PipelineConfig: return PipelineConfig( defaults_path=DEFAULTS_PATH, @@ -45,7 +45,7 @@ def config() -> PipelineConfig: ) -@pytest.fixture() +@pytest.fixture def handlers() -> ComponentHandlers: return ComponentHandlers( schema_handler=MagicMock(), diff --git a/tests/components/test_kafka_app.py b/tests/components/test_kafka_app.py index cb6953ff4..ff4b4a4f2 100644 --- a/tests/components/test_kafka_app.py +++ b/tests/components/test_kafka_app.py @@ -17,7 +17,7 @@ class TestKafkaApp: - @pytest.fixture() + @pytest.fixture def config(self) -> PipelineConfig: return PipelineConfig( defaults_path=DEFAULTS_PATH, @@ -25,7 +25,7 @@ def config(self) -> PipelineConfig: helm_diff_config=HelmDiffConfig(), ) - @pytest.fixture() + @pytest.fixture def handlers(self) -> ComponentHandlers: return ComponentHandlers( schema_handler=MagicMock(), diff --git a/tests/components/test_kafka_connector.py b/tests/components/test_kafka_connector.py index b9c2574ae..378707e8d 100644 --- a/tests/components/test_kafka_connector.py +++ b/tests/components/test_kafka_connector.py @@ -18,7 +18,7 @@ class TestKafkaConnector: - @pytest.fixture() + @pytest.fixture def config(self) -> PipelineConfig: return PipelineConfig( defaults_path=DEFAULTS_PATH, @@ -31,7 +31,7 @@ def config(self) -> PipelineConfig: helm_diff_config=HelmDiffConfig(), ) - @pytest.fixture() + @pytest.fixture def handlers(self) -> ComponentHandlers: return ComponentHandlers( schema_handler=AsyncMock(), @@ -45,13 +45,13 @@ def helm_mock(self, mocker: MockerFixture) -> MagicMock: "kpops.components.base_components.kafka_connector.Helm" ).return_value - @pytest.fixture() + @pytest.fixture def dry_run_handler(self, mocker: MockerFixture) -> MagicMock: return mocker.patch( "kpops.components.base_components.kafka_connector.DryRunHandler" ).return_value - @pytest.fixture() + @pytest.fixture def connector_config(self) -> KafkaConnectorConfig: return KafkaConnectorConfig( **{ @@ -79,7 +79,7 @@ def test_connector_config_name_override( name=CONNECTOR_NAME, config=config, handlers=handlers, - app={"connector.class": CONNECTOR_CLASS}, # type: ignore[reportGeneralTypeIssues] + app={"connector.class": CONNECTOR_CLASS}, # type: ignore namespace="test-namespace", ) assert connector.app.name == CONNECTOR_FULL_NAME @@ -91,7 +91,7 @@ def test_connector_config_name_override( name=CONNECTOR_NAME, config=config, handlers=handlers, - app={"connector.class": CONNECTOR_CLASS, "name": "different-name"}, # type: ignore[reportGeneralTypeIssues] + app={"connector.class": CONNECTOR_CLASS, "name": "different-name"}, # type: ignore namespace="test-namespace", ) @@ -102,6 +102,6 @@ def test_connector_config_name_override( name=CONNECTOR_NAME, config=config, handlers=handlers, - app={"connector.class": CONNECTOR_CLASS, "name": ""}, # type: ignore[reportGeneralTypeIssues] + app={"connector.class": CONNECTOR_CLASS, "name": ""}, # type: ignore namespace="test-namespace", ) diff --git a/tests/components/test_kafka_sink_connector.py b/tests/components/test_kafka_sink_connector.py index 7a8bcac26..4f9c0ca32 100644 --- a/tests/components/test_kafka_sink_connector.py +++ b/tests/components/test_kafka_sink_connector.py @@ -35,11 +35,11 @@ class TestKafkaSinkConnector(TestKafkaConnector): - @pytest.fixture() + @pytest.fixture def log_info_mock(self, mocker: MockerFixture) -> MagicMock: return mocker.patch("kpops.components.base_components.kafka_connector.log.info") - @pytest.fixture() + @pytest.fixture def connector( self, config: PipelineConfig, diff --git a/tests/components/test_kafka_source_connector.py b/tests/components/test_kafka_source_connector.py index 07c588382..1e40c7014 100644 --- a/tests/components/test_kafka_source_connector.py +++ b/tests/components/test_kafka_source_connector.py @@ -32,7 +32,7 @@ class TestKafkaSourceConnector(TestKafkaConnector): - @pytest.fixture() + @pytest.fixture def connector( self, config: PipelineConfig, diff --git a/tests/components/test_kubernetes_app.py b/tests/components/test_kubernetes_app.py index 075a44cb1..affd87cd9 100644 --- a/tests/components/test_kubernetes_app.py +++ b/tests/components/test_kubernetes_app.py @@ -27,7 +27,7 @@ class KubernetesTestValue(KubernetesAppConfig): class TestKubernetesApp: - @pytest.fixture() + @pytest.fixture def config(self) -> PipelineConfig: return PipelineConfig( defaults_path=DEFAULTS_PATH, @@ -35,7 +35,7 @@ def config(self) -> PipelineConfig: helm_diff_config=HelmDiffConfig(), ) - @pytest.fixture() + @pytest.fixture def handlers(self) -> ComponentHandlers: return ComponentHandlers( schema_handler=AsyncMock(), @@ -43,25 +43,25 @@ def handlers(self) -> ComponentHandlers: topic_handler=AsyncMock(), ) - @pytest.fixture() + @pytest.fixture def helm_mock(self, mocker: MockerFixture) -> MagicMock: return mocker.patch( "kpops.components.base_components.kubernetes_app.Helm" ).return_value - @pytest.fixture() + @pytest.fixture def log_info_mock(self, mocker: MockerFixture) -> MagicMock: return mocker.patch("kpops.components.base_components.kubernetes_app.log.info") - @pytest.fixture() + @pytest.fixture def app_value(self) -> KubernetesTestValue: return KubernetesTestValue(**{"name_override": "test-value"}) - @pytest.fixture() + @pytest.fixture def repo_config(self) -> HelmRepoConfig: return HelmRepoConfig(repository_name="test", url="https://bakdata.com") - @pytest.fixture() + @pytest.fixture def kubernetes_app( self, config: PipelineConfig, @@ -201,8 +201,8 @@ async def test_should_raise_not_implemented_error_when_helm_chart_is_not_set( helm_mock.add_repo.assert_called() assert ( - str(error.value) - == "Please implement the helm_chart property of the kpops.components.base_components.kubernetes_app module." + "Please implement the helm_chart property of the kpops.components.base_components.kubernetes_app module." + == str(error.value) ) @pytest.mark.asyncio() diff --git a/tests/components/test_producer_app.py b/tests/components/test_producer_app.py index cbea7b192..cdbc648f8 100644 --- a/tests/components/test_producer_app.py +++ b/tests/components/test_producer_app.py @@ -21,7 +21,7 @@ class TestProducerApp: PRODUCER_APP_NAME = "test-producer-app-with-long-name-0123456789abcdefghijklmnop" PRODUCER_APP_CLEAN_NAME = "test-producer-app-with-long-n-clean" - @pytest.fixture() + @pytest.fixture def handlers(self) -> ComponentHandlers: return ComponentHandlers( schema_handler=AsyncMock(), @@ -29,7 +29,7 @@ def handlers(self) -> ComponentHandlers: topic_handler=AsyncMock(), ) - @pytest.fixture() + @pytest.fixture def config(self) -> PipelineConfig: return PipelineConfig( defaults_path=DEFAULTS_PATH, @@ -40,7 +40,7 @@ def config(self) -> PipelineConfig: ), ) - @pytest.fixture() + @pytest.fixture def producer_app( self, config: PipelineConfig, handlers: ComponentHandlers ) -> ProducerApp: diff --git a/tests/components/test_streams_app.py b/tests/components/test_streams_app.py index 7e7259e1d..8570b925e 100644 --- a/tests/components/test_streams_app.py +++ b/tests/components/test_streams_app.py @@ -25,7 +25,7 @@ class TestStreamsApp: STREAMS_APP_NAME = "test-streams-app-with-long-name-0123456789abcdefghijklmnop" STREAMS_APP_CLEAN_NAME = "test-streams-app-with-long-na-clean" - @pytest.fixture() + @pytest.fixture def handlers(self) -> ComponentHandlers: return ComponentHandlers( schema_handler=AsyncMock(), @@ -33,7 +33,7 @@ def handlers(self) -> ComponentHandlers: topic_handler=AsyncMock(), ) - @pytest.fixture() + @pytest.fixture def config(self) -> PipelineConfig: return PipelineConfig( defaults_path=DEFAULTS_PATH, @@ -45,7 +45,7 @@ def config(self) -> PipelineConfig: helm_diff_config=HelmDiffConfig(), ) - @pytest.fixture() + @pytest.fixture def streams_app( self, config: PipelineConfig, handlers: ComponentHandlers ) -> StreamsApp: @@ -145,9 +145,7 @@ def test_no_empty_input_topic( def test_should_validate(self, config: PipelineConfig, handlers: ComponentHandlers): # An exception should be raised when both role and type are defined and type is input - with pytest.raises( - ValueError, match="Define role only if `type` is `pattern` or `None`" - ): + with pytest.raises(ValueError): StreamsApp( name=self.STREAMS_APP_NAME, config=config, @@ -169,9 +167,7 @@ def test_should_validate(self, config: PipelineConfig, handlers: ComponentHandle ) # An exception should be raised when both role and type are defined and type is error - with pytest.raises( - ValueError, match="Define `role` only if `type` is undefined" - ): + with pytest.raises(ValueError): StreamsApp( name=self.STREAMS_APP_NAME, config=config, diff --git a/tests/pipeline/test_components/components.py b/tests/pipeline/test_components/components.py index 86e2c8b8e..cb58d19f0 100644 --- a/tests/pipeline/test_components/components.py +++ b/tests/pipeline/test_components/components.py @@ -44,8 +44,7 @@ def inflate(self) -> list[PipelineComponent]: config=self.config, handlers=self.handlers, namespace="example-namespace", - # FIXME - app={ # type: ignore[reportGeneralTypeIssues] + app={ # type: ignore # FIXME "topics": topic_name, "transforms.changeTopic.replacement": f"{topic_name}-index-v1", }, @@ -65,7 +64,7 @@ def inflate(self) -> list[PipelineComponent]: name=f"{self.name}-inflated-streams-app", config=self.config, handlers=self.handlers, - to=ToSection( # type: ignore[reportGeneralTypeIssues] + to=ToSection( # type: ignore topics={ TopicName( f"{self.full_name}-" + "${component_name}" diff --git a/tests/pipeline/test_pipeline.py b/tests/pipeline/test_pipeline.py index 9d78ba7cc..d60d597cd 100644 --- a/tests/pipeline/test_pipeline.py +++ b/tests/pipeline/test_pipeline.py @@ -3,6 +3,7 @@ import pytest import yaml +from pytest import MonkeyPatch from snapshottest.module import SnapshotTest from typer.testing import CliRunner @@ -460,7 +461,7 @@ def test_default_config(self, snapshot: SnapshotTest): def test_env_vars_precedence_over_config( self, - monkeypatch: pytest.MonkeyPatch, + monkeypatch: MonkeyPatch, snapshot: SnapshotTest, ): monkeypatch.setenv(name="KPOPS_KAFKA_BROKERS", value="env_broker") diff --git a/tests/pipeline/test_template.py b/tests/pipeline/test_template.py index a43fbec5b..cd4436b7a 100644 --- a/tests/pipeline/test_template.py +++ b/tests/pipeline/test_template.py @@ -15,7 +15,7 @@ class TestTemplate: - @pytest.fixture() + @pytest.fixture def run_command(self, mocker: MockerFixture) -> MagicMock: return mocker.patch.object(Helm, "_Helm__execute") diff --git a/tests/utils/test_environment.py b/tests/utils/test_environment.py index 8fc02c826..09bbb75de 100644 --- a/tests/utils/test_environment.py +++ b/tests/utils/test_environment.py @@ -5,12 +5,12 @@ from kpops.utils.environment import Environment -@pytest.fixture() +@pytest.fixture def fake_environment_windows(): return {"MY": "fake", "ENVIRONMENT": "here"} -@pytest.fixture() +@pytest.fixture def fake_environment_linux(): return {"my": "fake", "environment": "here"}