From 352cf9c105e6de18e35aea6953ba0101d441f29a Mon Sep 17 00:00:00 2001 From: Ivan Yordanov Date: Mon, 23 Oct 2023 11:56:29 +0300 Subject: [PATCH] Fix early exit upon Helm exit code 1 (#376) fixes #373 ### Fix > I am getting this error when I try to use dry-run: subprocess.CalledProcessError: Command '['helm', 'get', 'manifest', 'account-producer', '--namespace', 'my-namespace']' returned non-zero exit status 1. why? does he launch it first in dry-run ? when I do that in my commandline I get To learn more, consult https://cloud.google.com/blog/products/containers-kubernetes/kubectl-auth-changes-in-gke Error: release: not found The [problem came from](https://github.com/bakdata/kpops/compare/2.0.9...2.0.10#diff-1ca7465daf9238dfe221304df636d96704ebf4935288b854a5c4c6b3ea9e3162R215) setting `subproces.run(..., check = True)` without handling the raised exception. ### Preventive measures I took some imports out of `TYPE_CHECKING` blocks to avoid problems with `pydantic` as it uses type hints at runtime ### Ruff Updated to the latest (`0.1.1`) version. Now all fixes deemed unsafe will not be automatically carried out even if it is possible to do so. The user can choose to enable the unsafe autofixes with the `--unsafe-fixes` flag ### Example While testing I noticed that we could use the namespace env var in some places in the example, so I did it and had to adjust the tests. --- .../bakdata/atm-fraud-detection/config.yaml | 4 +- .../bakdata/atm-fraud-detection/pipeline.yaml | 4 +- kpops/cli/pipeline_config.py | 9 ++--- kpops/component_handlers/helm_wrapper/helm.py | 2 +- .../base_components/kafka_connector.py | 6 +-- poetry.lock | 38 +++++++++---------- pyproject.toml | 3 +- tests/conftest.py | 16 ++++++++ tests/pipeline/snapshots/snap_test_example.py | 24 ++++++------ tests/pipeline/test_example.py | 2 + tests/pipeline/test_pipeline.py | 1 + 11 files changed, 61 insertions(+), 48 deletions(-) create mode 100644 tests/conftest.py diff --git a/examples/bakdata/atm-fraud-detection/config.yaml b/examples/bakdata/atm-fraud-detection/config.yaml index e3742ded9..d429723d6 100644 --- a/examples/bakdata/atm-fraud-detection/config.yaml +++ b/examples/bakdata/atm-fraud-detection/config.yaml @@ -4,9 +4,9 @@ topic_name_config: default_error_topic_name: "${pipeline_name}-${component_name}-dead-letter-topic" default_output_topic_name: "${pipeline_name}-${component_name}-topic" -brokers: "http://k8kafka-cp-kafka-headless.kpops.svc.cluster.local:9092" +brokers: "http://k8kafka-cp-kafka-headless.${NAMESPACE}.svc.cluster.local:9092" -schema_registry_url: "http://k8kafka-cp-schema-registry.kpops.svc.cluster.local:8081" +schema_registry_url: "http://k8kafka-cp-schema-registry.${NAMESPACE}.svc.cluster.local:8081" kafka_rest_host: "http://localhost:8082" diff --git a/examples/bakdata/atm-fraud-detection/pipeline.yaml b/examples/bakdata/atm-fraud-detection/pipeline.yaml index 9cf3610a2..9982aa0a7 100644 --- a/examples/bakdata/atm-fraud-detection/pipeline.yaml +++ b/examples/bakdata/atm-fraud-detection/pipeline.yaml @@ -84,14 +84,14 @@ connector.class: io.confluent.connect.jdbc.JdbcSinkConnector tasks.max: 1 topics: ${pipeline_name}-account-linker-topic - connection.url: jdbc:postgresql://postgresql-dev.kpops.svc.cluster.local:5432/app_db + connection.url: jdbc:postgresql://postgresql-dev.${NAMESPACE}.svc.cluster.local:5432/app_db connection.user: app1 connection.password: AppPassword connection.ds.pool.size: 5 insert.mode: insert insert.mode.databaselevel: true value.converter: io.confluent.connect.avro.AvroConverter - value.converter.schema.registry.url: http://k8kafka-cp-schema-registry.kpops.svc.cluster.local:8081 + value.converter.schema.registry.url: http://k8kafka-cp-schema-registry.${NAMESPACE}.svc.cluster.local:8081 key.converter: org.apache.kafka.connect.storage.StringConverter transforms: flatten transforms.flatten.type: org.apache.kafka.connect.transforms.Flatten$Value diff --git a/kpops/cli/pipeline_config.py b/kpops/cli/pipeline_config.py index 1400323f5..8ed19dfd8 100644 --- a/kpops/cli/pipeline_config.py +++ b/kpops/cli/pipeline_config.py @@ -1,18 +1,15 @@ from __future__ import annotations +from collections.abc import Callable from pathlib import Path -from typing import TYPE_CHECKING, Any +from typing import Any from pydantic import BaseConfig, BaseSettings, Field +from pydantic.env_settings import SettingsSourceCallable from kpops.component_handlers.helm_wrapper.model import HelmConfig, HelmDiffConfig from kpops.utils.yaml_loading import load_yaml_file -if TYPE_CHECKING: - from collections.abc import Callable - - from pydantic.env_settings import SettingsSourceCallable - ENV_PREFIX = "KPOPS_" diff --git a/kpops/component_handlers/helm_wrapper/helm.py b/kpops/component_handlers/helm_wrapper/helm.py index b1b101b41..5e4d758db 100644 --- a/kpops/component_handlers/helm_wrapper/helm.py +++ b/kpops/component_handlers/helm_wrapper/helm.py @@ -212,7 +212,7 @@ def __execute(self, command: list[str]) -> str: log.debug(f"Executing {' '.join(command)}") process = subprocess.run( command, - check=True, + check=False, capture_output=True, text=True, ) diff --git a/kpops/components/base_components/kafka_connector.py b/kpops/components/base_components/kafka_connector.py index f77588401..fa6e83f1c 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,13 +25,11 @@ 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") diff --git a/poetry.lock b/poetry.lock index 9a50b9ae1..d901baf75 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1385,28 +1385,28 @@ jupyter = ["ipywidgets (>=7.5.1,<8.0.0)"] [[package]] name = "ruff" -version = "0.0.292" +version = "0.1.1" description = "An extremely fast Python linter, written in Rust." optional = false python-versions = ">=3.7" files = [ - {file = "ruff-0.0.292-py3-none-macosx_10_7_x86_64.whl", hash = "sha256:02f29db018c9d474270c704e6c6b13b18ed0ecac82761e4fcf0faa3728430c96"}, - {file = "ruff-0.0.292-py3-none-macosx_10_9_x86_64.macosx_11_0_arm64.macosx_10_9_universal2.whl", hash = "sha256:69654e564342f507edfa09ee6897883ca76e331d4bbc3676d8a8403838e9fade"}, - {file = "ruff-0.0.292-py3-none-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:6c3c91859a9b845c33778f11902e7b26440d64b9d5110edd4e4fa1726c41e0a4"}, - {file = "ruff-0.0.292-py3-none-manylinux_2_17_armv7l.manylinux2014_armv7l.whl", hash = "sha256:f4476f1243af2d8c29da5f235c13dca52177117935e1f9393f9d90f9833f69e4"}, - {file = "ruff-0.0.292-py3-none-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:be8eb50eaf8648070b8e58ece8e69c9322d34afe367eec4210fdee9a555e4ca7"}, - {file = "ruff-0.0.292-py3-none-manylinux_2_17_ppc64.manylinux2014_ppc64.whl", hash = "sha256:9889bac18a0c07018aac75ef6c1e6511d8411724d67cb879103b01758e110a81"}, - {file = "ruff-0.0.292-py3-none-manylinux_2_17_ppc64le.manylinux2014_ppc64le.whl", hash = "sha256:6bdfabd4334684a4418b99b3118793f2c13bb67bf1540a769d7816410402a205"}, - {file = "ruff-0.0.292-py3-none-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:aa7c77c53bfcd75dbcd4d1f42d6cabf2485d2e1ee0678da850f08e1ab13081a8"}, - {file = "ruff-0.0.292-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:8e087b24d0d849c5c81516ec740bf4fd48bf363cfb104545464e0fca749b6af9"}, - {file = "ruff-0.0.292-py3-none-musllinux_1_2_aarch64.whl", hash = "sha256:f160b5ec26be32362d0774964e218f3fcf0a7da299f7e220ef45ae9e3e67101a"}, - {file = "ruff-0.0.292-py3-none-musllinux_1_2_armv7l.whl", hash = "sha256:ac153eee6dd4444501c4bb92bff866491d4bfb01ce26dd2fff7ca472c8df9ad0"}, - {file = "ruff-0.0.292-py3-none-musllinux_1_2_i686.whl", hash = "sha256:87616771e72820800b8faea82edd858324b29bb99a920d6aa3d3949dd3f88fb0"}, - {file = "ruff-0.0.292-py3-none-musllinux_1_2_x86_64.whl", hash = "sha256:b76deb3bdbea2ef97db286cf953488745dd6424c122d275f05836c53f62d4016"}, - {file = "ruff-0.0.292-py3-none-win32.whl", hash = "sha256:e854b05408f7a8033a027e4b1c7f9889563dd2aca545d13d06711e5c39c3d003"}, - {file = "ruff-0.0.292-py3-none-win_amd64.whl", hash = "sha256:f27282bedfd04d4c3492e5c3398360c9d86a295be00eccc63914438b4ac8a83c"}, - {file = "ruff-0.0.292-py3-none-win_arm64.whl", hash = "sha256:7f67a69c8f12fbc8daf6ae6d36705037bde315abf8b82b6e1f4c9e74eb750f68"}, - {file = "ruff-0.0.292.tar.gz", hash = "sha256:1093449e37dd1e9b813798f6ad70932b57cf614e5c2b5c51005bf67d55db33ac"}, + {file = "ruff-0.1.1-py3-none-macosx_10_7_x86_64.whl", hash = "sha256:b7cdc893aef23ccc14c54bd79a8109a82a2c527e11d030b62201d86f6c2b81c5"}, + {file = "ruff-0.1.1-py3-none-macosx_10_9_x86_64.macosx_11_0_arm64.macosx_10_9_universal2.whl", hash = "sha256:620d4b34302538dbd8bbbe8fdb8e8f98d72d29bd47e972e2b59ce6c1e8862257"}, + {file = "ruff-0.1.1-py3-none-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:2a909d3930afdbc2e9fd893b0034479e90e7981791879aab50ce3d9f55205bd6"}, + {file = "ruff-0.1.1-py3-none-manylinux_2_17_armv7l.manylinux2014_armv7l.whl", hash = "sha256:3305d1cb4eb8ff6d3e63a48d1659d20aab43b49fe987b3ca4900528342367145"}, + {file = "ruff-0.1.1-py3-none-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:c34ae501d0ec71acf19ee5d4d889e379863dcc4b796bf8ce2934a9357dc31db7"}, + {file = "ruff-0.1.1-py3-none-manylinux_2_17_ppc64.manylinux2014_ppc64.whl", hash = "sha256:6aa7e63c3852cf8fe62698aef31e563e97143a4b801b57f920012d0e07049a8d"}, + {file = "ruff-0.1.1-py3-none-manylinux_2_17_ppc64le.manylinux2014_ppc64le.whl", hash = "sha256:2d68367d1379a6b47e61bc9de144a47bcdb1aad7903bbf256e4c3d31f11a87ae"}, + {file = "ruff-0.1.1-py3-none-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:bc11955f6ce3398d2afe81ad7e49d0ebf0a581d8bcb27b8c300281737735e3a3"}, + {file = "ruff-0.1.1-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:cbbd8eead88ea83a250499074e2a8e9d80975f0b324b1e2e679e4594da318c25"}, + {file = "ruff-0.1.1-py3-none-musllinux_1_2_aarch64.whl", hash = "sha256:f4780e2bb52f3863a565ec3f699319d3493b83ff95ebbb4993e59c62aaf6e75e"}, + {file = "ruff-0.1.1-py3-none-musllinux_1_2_armv7l.whl", hash = "sha256:8f5b24daddf35b6c207619301170cae5d2699955829cda77b6ce1e5fc69340df"}, + {file = "ruff-0.1.1-py3-none-musllinux_1_2_i686.whl", hash = "sha256:d3f9ac658ba29e07b95c80fa742b059a55aefffa8b1e078bc3c08768bdd4b11a"}, + {file = "ruff-0.1.1-py3-none-musllinux_1_2_x86_64.whl", hash = "sha256:3521bf910104bf781e6753282282acc145cbe3eff79a1ce6b920404cd756075a"}, + {file = "ruff-0.1.1-py3-none-win32.whl", hash = "sha256:ba3208543ab91d3e4032db2652dcb6c22a25787b85b8dc3aeff084afdc612e5c"}, + {file = "ruff-0.1.1-py3-none-win_amd64.whl", hash = "sha256:3ff3006c97d9dc396b87fb46bb65818e614ad0181f059322df82bbfe6944e264"}, + {file = "ruff-0.1.1-py3-none-win_arm64.whl", hash = "sha256:e140bd717c49164c8feb4f65c644046fe929c46f42493672853e3213d7bdbce2"}, + {file = "ruff-0.1.1.tar.gz", hash = "sha256:c90461ae4abec261609e5ea436de4a4b5f2822921cf04c16d2cc9327182dbbcc"}, ] [[package]] @@ -1769,4 +1769,4 @@ watchmedo = ["PyYAML (>=3.10)"] [metadata] lock-version = "2.0" python-versions = "^3.10" -content-hash = "056b014fc985bda3ac9d33518eae39b228f776e84307ee4ddd28bd330a1c36e6" +content-hash = "c90b6d135b493146da2d248ed7c20e717051bed7ddf222087fe7aa52633f8682" diff --git a/pyproject.toml b/pyproject.toml index 64b573128..84ff5d74b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -45,7 +45,7 @@ pytest-mock = "^3.10.0" pytest-timeout = "^2.1.0" snapshottest = "^0.6.0" pre-commit = "^2.19.0" -ruff = "^0.0.292" +ruff = "^0.1.1" black = "^23.7.0" typer-cli = "^0.0.13" pyright = "^1.1.314" @@ -135,7 +135,6 @@ select = [ "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 diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 000000000..cb88c2294 --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,16 @@ +import os +from collections.abc import Iterator +from unittest import mock + +import pytest + + +@pytest.fixture() +def mock_env() -> Iterator[os._Environ[str]]: + """Clear ``os.environ``. + + :yield: ``os.environ``. Prevents the function and the mock + context from exiting. + """ + with mock.patch.dict(os.environ, clear=True): + yield os.environ diff --git a/tests/pipeline/snapshots/snap_test_example.py b/tests/pipeline/snapshots/snap_test_example.py index cff924b5f..95d63ab70 100644 --- a/tests/pipeline/snapshots/snap_test_example.py +++ b/tests/pipeline/snapshots/snap_test_example.py @@ -23,12 +23,12 @@ 'replicaCount': 1, 'schedule': '0 12 * * *', 'streams': { - 'brokers': 'http://k8kafka-cp-kafka-headless.kpops.svc.cluster.local:9092', + 'brokers': 'http://k8kafka-cp-kafka-headless.${NAMESPACE}.svc.cluster.local:9092', 'extraOutputTopics': { }, 'optimizeLeaveGroupBehavior': False, 'outputTopic': 'bakdata-atm-fraud-detection-account-producer-topic', - 'schemaRegistryUrl': 'http://k8kafka-cp-schema-registry.kpops.svc.cluster.local:8081' + 'schemaRegistryUrl': 'http://k8kafka-cp-schema-registry.${NAMESPACE}.svc.cluster.local:8081' }, 'suspend': True }, @@ -74,12 +74,12 @@ 'replicaCount': 1, 'schedule': '0 12 * * *', 'streams': { - 'brokers': 'http://k8kafka-cp-kafka-headless.kpops.svc.cluster.local:9092', + 'brokers': 'http://k8kafka-cp-kafka-headless.${NAMESPACE}.svc.cluster.local:9092', 'extraOutputTopics': { }, 'optimizeLeaveGroupBehavior': False, 'outputTopic': 'bakdata-atm-fraud-detection-transaction-avro-producer-topic', - 'schemaRegistryUrl': 'http://k8kafka-cp-schema-registry.kpops.svc.cluster.local:8081' + 'schemaRegistryUrl': 'http://k8kafka-cp-schema-registry.${NAMESPACE}.svc.cluster.local:8081' }, 'suspend': True }, @@ -129,14 +129,14 @@ }, 'replicaCount': 1, 'streams': { - 'brokers': 'http://k8kafka-cp-kafka-headless.kpops.svc.cluster.local:9092', + 'brokers': 'http://k8kafka-cp-kafka-headless.${NAMESPACE}.svc.cluster.local:9092', 'errorTopic': 'bakdata-atm-fraud-detection-transaction-joiner-dead-letter-topic', 'inputTopics': [ 'bakdata-atm-fraud-detection-transaction-avro-producer-topic' ], 'optimizeLeaveGroupBehavior': False, 'outputTopic': 'bakdata-atm-fraud-detection-transaction-joiner-topic', - 'schemaRegistryUrl': 'http://k8kafka-cp-schema-registry.kpops.svc.cluster.local:8081' + 'schemaRegistryUrl': 'http://k8kafka-cp-schema-registry.${NAMESPACE}.svc.cluster.local:8081' } }, 'name': 'transaction-joiner', @@ -191,14 +191,14 @@ }, 'replicaCount': 1, 'streams': { - 'brokers': 'http://k8kafka-cp-kafka-headless.kpops.svc.cluster.local:9092', + 'brokers': 'http://k8kafka-cp-kafka-headless.${NAMESPACE}.svc.cluster.local:9092', 'errorTopic': 'bakdata-atm-fraud-detection-fraud-detector-dead-letter-topic', 'inputTopics': [ 'bakdata-atm-fraud-detection-transaction-joiner-topic' ], 'optimizeLeaveGroupBehavior': False, 'outputTopic': 'bakdata-atm-fraud-detection-fraud-detector-topic', - 'schemaRegistryUrl': 'http://k8kafka-cp-schema-registry.kpops.svc.cluster.local:8081' + 'schemaRegistryUrl': 'http://k8kafka-cp-schema-registry.${NAMESPACE}.svc.cluster.local:8081' } }, 'name': 'fraud-detector', @@ -253,7 +253,7 @@ }, 'replicaCount': 1, 'streams': { - 'brokers': 'http://k8kafka-cp-kafka-headless.kpops.svc.cluster.local:9092', + 'brokers': 'http://k8kafka-cp-kafka-headless.${NAMESPACE}.svc.cluster.local:9092', 'errorTopic': 'bakdata-atm-fraud-detection-account-linker-dead-letter-topic', 'extraInputTopics': { 'accounts': [ @@ -265,7 +265,7 @@ ], 'optimizeLeaveGroupBehavior': False, 'outputTopic': 'bakdata-atm-fraud-detection-account-linker-topic', - 'schemaRegistryUrl': 'http://k8kafka-cp-schema-registry.kpops.svc.cluster.local:8081' + 'schemaRegistryUrl': 'http://k8kafka-cp-schema-registry.${NAMESPACE}.svc.cluster.local:8081' } }, 'from': { @@ -315,7 +315,7 @@ 'auto.create': True, 'connection.ds.pool.size': 5, 'connection.password': 'AppPassword', - 'connection.url': 'jdbc:postgresql://postgresql-dev.kpops.svc.cluster.local:5432/app_db', + 'connection.url': 'jdbc:postgresql://postgresql-dev.${NAMESPACE}.svc.cluster.local:5432/app_db', 'connection.user': 'app1', 'connector.class': 'io.confluent.connect.jdbc.JdbcSinkConnector', 'errors.deadletterqueue.context.headers.enable': True, @@ -333,7 +333,7 @@ 'transforms': 'flatten', 'transforms.flatten.type': 'org.apache.kafka.connect.transforms.Flatten$Value', 'value.converter': 'io.confluent.connect.avro.AvroConverter', - 'value.converter.schema.registry.url': 'http://k8kafka-cp-schema-registry.kpops.svc.cluster.local:8081' + 'value.converter.schema.registry.url': 'http://k8kafka-cp-schema-registry.${NAMESPACE}.svc.cluster.local:8081' }, 'name': 'postgresql-connector', 'namespace': '${NAMESPACE}', diff --git a/tests/pipeline/test_example.py b/tests/pipeline/test_example.py index ea2b27c43..e3b3e5286 100644 --- a/tests/pipeline/test_example.py +++ b/tests/pipeline/test_example.py @@ -1,3 +1,4 @@ +import pytest import yaml from snapshottest.module import SnapshotTest from typer.testing import CliRunner @@ -7,6 +8,7 @@ runner = CliRunner() +@pytest.mark.usefixtures("mock_env") class TestExample: def test_atm_fraud(self, snapshot: SnapshotTest): result = runner.invoke( diff --git a/tests/pipeline/test_pipeline.py b/tests/pipeline/test_pipeline.py index 433960e74..bc584dbfa 100644 --- a/tests/pipeline/test_pipeline.py +++ b/tests/pipeline/test_pipeline.py @@ -16,6 +16,7 @@ PIPELINE_BASE_DIR_PATH = RESOURCE_PATH.parent +@pytest.mark.usefixtures("mock_env") class TestPipeline: def test_python_api(self): pipeline = kpops.generate(