From 2a873ffeb2471dd41c8d2277f6cfa5aca3d65c4e Mon Sep 17 00:00:00 2001 From: Alejandro Jaramillo Date: Mon, 24 Jul 2023 13:56:41 +0200 Subject: [PATCH 01/23] Save progress --- kpops/cli/main.py | 11 ++- .../kafka_connect/connect_wrapper.py | 1 + .../component_handlers/topic/proxy_wrapper.py | 87 +++++++++++-------- kpops/components/base_components/kafka_app.py | 4 +- .../base_components/kafka_connector.py | 2 +- .../base_components/kubernetes_app.py | 2 +- .../base_components/pipeline_component.py | 2 +- kpops/utils/async_utils.py | 5 ++ poetry.lock | 84 ++++++++++++------ pyproject.toml | 8 ++ .../topic/test_proxy_wrapper.py | 27 +++--- tests/components/test_kafka_app.py | 5 +- tests/components/test_kafka_sink_connector.py | 5 +- .../components/test_kafka_source_connector.py | 5 +- tests/components/test_kubernetes_app.py | 15 ++-- tests/components/test_producer_app.py | 5 +- tests/components/test_streams_app.py | 5 +- 17 files changed, 176 insertions(+), 97 deletions(-) create mode 100644 kpops/utils/async_utils.py diff --git a/kpops/cli/main.py b/kpops/cli/main.py index 71d3cc8d3..00be4b5f6 100644 --- a/kpops/cli/main.py +++ b/kpops/cli/main.py @@ -1,5 +1,6 @@ from __future__ import annotations +import asyncio import logging from pathlib import Path from typing import TYPE_CHECKING, Iterator, Optional @@ -275,9 +276,13 @@ def deploy( ) steps_to_apply = get_steps_to_apply(pipeline, steps) - for component in steps_to_apply: - log_action("Deploy", component) - component.deploy(dry_run) + + async def async_deploy(): + for component in steps_to_apply: + log_action("Deploy", component) + component.deploy(dry_run) + + asyncio.run(async_deploy()) @app.command(help="Destroy pipeline steps") diff --git a/kpops/component_handlers/kafka_connect/connect_wrapper.py b/kpops/component_handlers/kafka_connect/connect_wrapper.py index 4fac328ad..57f7c2653 100644 --- a/kpops/component_handlers/kafka_connect/connect_wrapper.py +++ b/kpops/component_handlers/kafka_connect/connect_wrapper.py @@ -50,6 +50,7 @@ def create_connector( """ config_json = kafka_connect_config.dict(exclude_none=True) connect_data = {"name": connector_name, "config": config_json} + response = requests.post( url=f"{self._host}/connectors", headers=HEADERS, json=connect_data ) diff --git a/kpops/component_handlers/topic/proxy_wrapper.py b/kpops/component_handlers/topic/proxy_wrapper.py index d3497f15a..7652a7e3f 100644 --- a/kpops/component_handlers/topic/proxy_wrapper.py +++ b/kpops/component_handlers/topic/proxy_wrapper.py @@ -1,7 +1,8 @@ import logging -from functools import cached_property +import httpx import requests +from async_property import async_cached_property from kpops.cli.pipeline_config import PipelineConfig from kpops.component_handlers.topic.exception import ( @@ -33,8 +34,8 @@ def __init__(self, pipeline_config: PipelineConfig) -> None: self._host = pipeline_config.kafka_rest_host - @cached_property - def cluster_id(self) -> str: + @async_cached_property + async def cluster_id(self) -> str: """ Gets the Kafka cluster ID by sending a requests to Kafka REST proxy. More information about the cluster ID can be found here: @@ -44,7 +45,9 @@ def cluster_id(self) -> str: bootstrap.servers configuration. Therefore, only one Kafka cluster will be returned. :return: The Kafka cluster ID. """ - response = requests.get(url=f"{self._host}/v3/clusters") + client = httpx.AsyncClient() + response = await client.get(url=f"{self._host}/v3/clusters") + await client.aclose() if response.status_code == requests.status_codes.codes.ok: cluster_information = response.json() return cluster_information["data"][0]["cluster_id"] @@ -55,17 +58,19 @@ def cluster_id(self) -> str: def host(self) -> str: return self._host - def create_topic(self, topic_spec: TopicSpec) -> None: + async def create_topic(self, topic_spec: TopicSpec) -> None: """ Creates a topic. API Reference: https://docs.confluent.io/platform/current/kafka-rest/api.html#post--clusters-cluster_id-topics :param topic_spec: The topic specification. """ - response = requests.post( + client = httpx.AsyncClient() + response = await client.post( url=f"{self._host}/v3/clusters/{self.cluster_id}/topics", headers=HEADERS, json=topic_spec.dict(exclude_none=True), ) + await client.aclose() if response.status_code == requests.status_codes.codes.created: log.info(f"Topic {topic_spec.topic_name} created.") log.debug(response.json()) @@ -73,59 +78,66 @@ def create_topic(self, topic_spec: TopicSpec) -> None: raise KafkaRestProxyError(response) - def delete_topic(self, topic_name: str) -> None: + async def delete_topic(self, topic_name: str) -> None: """ Deletes a topic API Reference: https://docs.confluent.io/platform/current/kafka-rest/api.html#delete--clusters-cluster_id-topics-topic_name :param topic_name: Name of the topic """ - response = requests.delete( + + client = httpx.AsyncClient() + response = await client.delete( url=f"{self.host}/v3/clusters/{self.cluster_id}/topics/{topic_name}", headers=HEADERS, ) + await client.aclose() if response.status_code == requests.status_codes.codes.no_content: log.info(f"Topic {topic_name} deleted.") return raise KafkaRestProxyError(response) - def get_topic(self, topic_name: str) -> TopicResponse: + async def get_topic(self, topic_name: str) -> TopicResponse: """ Returns the topic with the given topic_name. API Reference: https://docs.confluent.io/platform/current/kafka-rest/api.html#get--clusters-cluster_id-topics-topic_name :param topic_name: The topic name. :return: Response of the get topic API """ - response = requests.get( - url=f"{self.host}/v3/clusters/{self.cluster_id}/topics/{topic_name}", - headers=HEADERS, - ) - if response.status_code == requests.status_codes.codes.ok: - log.debug(f"Topic {topic_name} found.") - log.debug(response.json()) - return TopicResponse(**response.json()) - - elif ( - response.status_code == requests.status_codes.codes.not_found - and response.json()["error_code"] == 40403 - ): - log.debug(f"Topic {topic_name} not found.") - log.debug(response.json()) - raise TopicNotFoundException() - - raise KafkaRestProxyError(response) - - def get_topic_config(self, topic_name: str) -> TopicConfigResponse: + async with httpx.AsyncClient() as client: + response = await client.get( + url=f"{self.host}/v3/clusters/{self.cluster_id}/topics/{topic_name}", + headers=HEADERS, + ) + if response.status_code == requests.status_codes.codes.ok: + log.debug(f"Topic {topic_name} found.") + log.debug(response.json()) + return TopicResponse(**response.json()) + + elif ( + response.status_code == requests.status_codes.codes.not_found + and response.json()["error_code"] == 40403 + ): + log.debug(f"Topic {topic_name} not found.") + log.debug(response.json()) + raise TopicNotFoundException() + + raise KafkaRestProxyError(response) + + async def get_topic_config(self, topic_name: str) -> TopicConfigResponse: """ Return the config with the given topic_name. API Reference: https://docs.confluent.io/platform/current/kafka-rest/api.html#acl-v3 :param topic_name: The topic name. :return: The topic configuration. """ - response = requests.get( + + client = httpx.AsyncClient() + response = await client.get( url=f"{self.host}/v3/clusters/{self.cluster_id}/topics/{topic_name}/configs", headers=HEADERS, ) + await client.aclose() if response.status_code == requests.status_codes.codes.ok: log.debug(f"Configs for {topic_name} found.") @@ -142,34 +154,41 @@ def get_topic_config(self, topic_name: str) -> TopicConfigResponse: raise KafkaRestProxyError(response) - def batch_alter_topic_config(self, topic_name: str, json_body: list[dict]) -> None: + async def batch_alter_topic_config( + self, topic_name: str, json_body: list[dict] + ) -> None: """ Reset config of given config_name param to the default value on the kafka server. API Reference: https://docs.confluent.io/platform/current/kafka-rest/api.html#post--clusters-cluster_id-topics-topic_name-configs-alter :param topic_name: The topic name. :param config_name: The configuration parameter name. """ - response = requests.post( + client = httpx.AsyncClient() + response = await client.post( url=f"{self.host}/v3/clusters/{self.cluster_id}/topics/{topic_name}/configs:alter", headers=HEADERS, json={"data": json_body}, ) + await client.aclose() + if response.status_code == requests.status_codes.codes.no_content: log.info(f"Config of topic {topic_name} was altered.") return raise KafkaRestProxyError(response) - def get_broker_config(self) -> BrokerConfigResponse: + async def get_broker_config(self) -> BrokerConfigResponse: """ Return the list of configuration parameters for all the brokers in the given Kafka cluster. API Reference: https://docs.confluent.io/platform/current/kafka-rest/api.html#get--clusters-cluster_id-brokers---configs :return: The broker configuration. """ - response = requests.get( + client = httpx.AsyncClient() + response = await client.get( url=f"{self.host}/v3/clusters/{self.cluster_id}/brokers/-/configs", headers=HEADERS, ) + await client.aclose() if response.status_code == requests.status_codes.codes.ok: log.debug("Broker configs found.") diff --git a/kpops/components/base_components/kafka_app.py b/kpops/components/base_components/kafka_app.py index de74da19e..889aeaa40 100644 --- a/kpops/components/base_components/kafka_app.py +++ b/kpops/components/base_components/kafka_app.py @@ -105,7 +105,7 @@ def clean_up_helm_chart(self) -> str: raise NotImplementedError() @override - def deploy(self, dry_run: bool) -> None: + async def deploy(self, dry_run: bool) -> None: if self.to: self.handlers.topic_handler.create_topics( to_section=self.to, dry_run=dry_run @@ -115,7 +115,7 @@ def deploy(self, dry_run: bool) -> None: self.handlers.schema_handler.submit_schemas( to_section=self.to, dry_run=dry_run ) - super().deploy(dry_run) + await super().deploy(dry_run) def _run_clean_up_job( self, diff --git a/kpops/components/base_components/kafka_connector.py b/kpops/components/base_components/kafka_connector.py index 6f5bdeb7d..275ba4797 100644 --- a/kpops/components/base_components/kafka_connector.py +++ b/kpops/components/base_components/kafka_connector.py @@ -120,7 +120,7 @@ def kafka_connect_resetter_chart(self) -> str: return f"{self.repo_config.repository_name}/kafka-connect-resetter" @override - def deploy(self, dry_run: bool) -> None: + async def deploy(self, dry_run: bool) -> None: if self.to: self.handlers.topic_handler.create_topics( to_section=self.to, dry_run=dry_run diff --git a/kpops/components/base_components/kubernetes_app.py b/kpops/components/base_components/kubernetes_app.py index b9fa9f4c9..d64f345bd 100644 --- a/kpops/components/base_components/kubernetes_app.py +++ b/kpops/components/base_components/kubernetes_app.py @@ -126,7 +126,7 @@ def template( print(stdout) @override - def deploy(self, dry_run: bool) -> None: + async def deploy(self, dry_run: bool) -> None: stdout = self.helm.upgrade_install( self.helm_release_name, self.get_helm_chart(), diff --git a/kpops/components/base_components/pipeline_component.py b/kpops/components/base_components/pipeline_component.py index 7fe5e59c1..2d9dcb0b1 100644 --- a/kpops/components/base_components/pipeline_component.py +++ b/kpops/components/base_components/pipeline_component.py @@ -239,7 +239,7 @@ def template( :type cert_file: str, optional """ - def deploy(self, dry_run: bool) -> None: + async def deploy(self, dry_run: bool) -> None: """Deploy the component (self) to the k8s cluster :param dry_run: Whether to do a dry run of the command diff --git a/kpops/utils/async_utils.py b/kpops/utils/async_utils.py new file mode 100644 index 000000000..2b71286bb --- /dev/null +++ b/kpops/utils/async_utils.py @@ -0,0 +1,5 @@ +import asyncio + + +def get_loop(): + return asyncio.get_running_loop() diff --git a/poetry.lock b/poetry.lock index 08009d83f..94b8394c2 100644 --- a/poetry.lock +++ b/poetry.lock @@ -2,13 +2,13 @@ [[package]] name = "aiofiles" -version = "22.1.0" +version = "23.1.0" description = "File support for asyncio." optional = false python-versions = ">=3.7,<4.0" files = [ - {file = "aiofiles-22.1.0-py3-none-any.whl", hash = "sha256:1142fa8e80dbae46bb6339573ad4c8c0841358f79c6eb50a493dceca14621bad"}, - {file = "aiofiles-22.1.0.tar.gz", hash = "sha256:9107f1ca0b2a5553987a94a3c9959fe5b491fdf731389aa5b7b1bd0733e32de6"}, + {file = "aiofiles-23.1.0-py3-none-any.whl", hash = "sha256:9312414ae06472eb6f1d163f555e466a23aed1c8f60c30cccf7121dba2e53eb2"}, + {file = "aiofiles-23.1.0.tar.gz", hash = "sha256:edd247df9a19e0db16534d4baaf536d6609a43e1de5401d7a4c1c148753a1635"}, ] [[package]] @@ -31,6 +31,17 @@ doc = ["packaging", "sphinx-autodoc-typehints (>=1.2.0)", "sphinx-rtd-theme"] test = ["contextlib2", "coverage[toml] (>=4.5)", "hypothesis (>=4.0)", "mock (>=4)", "pytest (>=7.0)", "pytest-mock (>=3.6.1)", "trustme", "uvloop (<0.15)", "uvloop (>=0.15)"] trio = ["trio (>=0.16,<0.22)"] +[[package]] +name = "async-property" +version = "0.2.2" +description = "Python decorator for async properties." +optional = false +python-versions = "*" +files = [ + {file = "async_property-0.2.2-py2.py3-none-any.whl", hash = "sha256:8924d792b5843994537f8ed411165700b27b2bd966cefc4daeefc1253442a9d7"}, + {file = "async_property-0.2.2.tar.gz", hash = "sha256:17d9bd6ca67e27915a75d92549df64b5c7174e9dc806b30a3934dc4ff0506380"}, +] + [[package]] name = "attrs" version = "22.1.0" @@ -372,24 +383,24 @@ socks = ["socksio (==1.*)"] [[package]] name = "httpx" -version = "0.23.1" +version = "0.24.1" description = "The next generation HTTP client." optional = false python-versions = ">=3.7" files = [ - {file = "httpx-0.23.1-py3-none-any.whl", hash = "sha256:0b9b1f0ee18b9978d637b0776bfd7f54e2ca278e063e3586d8f01cda89e042a8"}, - {file = "httpx-0.23.1.tar.gz", hash = "sha256:202ae15319be24efe9a8bd4ed4360e68fde7b38bcc2ce87088d416f026667d19"}, + {file = "httpx-0.24.1-py3-none-any.whl", hash = "sha256:06781eb9ac53cde990577af654bd990a4949de37a28bdb4a230d434f3a30b9bd"}, + {file = "httpx-0.24.1.tar.gz", hash = "sha256:5853a43053df830c20f8110c5e69fe44d035d850b2dfe795e196f00fdb774bdd"}, ] [package.dependencies] certifi = "*" -httpcore = ">=0.15.0,<0.17.0" -rfc3986 = {version = ">=1.3,<2", extras = ["idna2008"]} +httpcore = ">=0.15.0,<0.18.0" +idna = "*" sniffio = "*" [package.extras] brotli = ["brotli", "brotlicffi"] -cli = ["click (==8.*)", "pygments (==2.*)", "rich (>=10,<13)"] +cli = ["click (==8.*)", "pygments (==2.*)", "rich (>=10,<14)"] http2 = ["h2 (>=3,<5)"] socks = ["socksio (==1.*)"] @@ -966,6 +977,42 @@ tomli = {version = ">=1.0.0", markers = "python_version < \"3.11\""} [package.extras] testing = ["argcomplete", "hypothesis (>=3.56)", "mock", "nose", "pygments (>=2.7.2)", "requests", "xmlschema"] +[[package]] +name = "pytest-asyncio" +version = "0.21.1" +description = "Pytest support for asyncio" +optional = false +python-versions = ">=3.7" +files = [ + {file = "pytest-asyncio-0.21.1.tar.gz", hash = "sha256:40a7eae6dded22c7b604986855ea48400ab15b069ae38116e8c01238e9eeb64d"}, + {file = "pytest_asyncio-0.21.1-py3-none-any.whl", hash = "sha256:8666c1c8ac02631d7c51ba282e0c69a8a452b211ffedf2599099845da5c5c37b"}, +] + +[package.dependencies] +pytest = ">=7.0.0" + +[package.extras] +docs = ["sphinx (>=5.3)", "sphinx-rtd-theme (>=1.0)"] +testing = ["coverage (>=6.2)", "flaky (>=3.5.0)", "hypothesis (>=5.7.1)", "mypy (>=0.931)", "pytest-trio (>=0.7.0)"] + +[[package]] +name = "pytest-httpx" +version = "0.22.0" +description = "Send responses to httpx." +optional = false +python-versions = ">=3.7" +files = [ + {file = "pytest_httpx-0.22.0-py3-none-any.whl", hash = "sha256:cefb7dcf66a4cb0601b0de05e576cca423b6081f3245e7912a4d84c58fa3eae8"}, + {file = "pytest_httpx-0.22.0.tar.gz", hash = "sha256:3a82797f3a9a14d51e8c6b7fa97524b68b847ee801109c062e696b4744f4431c"}, +] + +[package.dependencies] +httpx = "==0.24.*" +pytest = ">=6.0,<8.0" + +[package.extras] +testing = ["pytest-asyncio (==0.20.*)", "pytest-cov (==4.*)"] + [[package]] name = "pytest-mock" version = "3.10.0" @@ -1262,23 +1309,6 @@ urllib3 = ">=1.25.10" [package.extras] tests = ["coverage (>=6.0.0)", "flake8", "mypy", "pytest (>=7.0.0)", "pytest-asyncio", "pytest-cov", "pytest-httpserver", "types-requests"] -[[package]] -name = "rfc3986" -version = "1.5.0" -description = "Validating URI References per RFC 3986" -optional = false -python-versions = "*" -files = [ - {file = "rfc3986-1.5.0-py2.py3-none-any.whl", hash = "sha256:a86d6e1f5b1dc238b218b012df0aa79409667bb209e58da56d0b94704e712a97"}, - {file = "rfc3986-1.5.0.tar.gz", hash = "sha256:270aaf10d87d0d4e095063c65bf3ddbc6ee3d0b226328ce21e036f946e421835"}, -] - -[package.dependencies] -idna = {version = "*", optional = true, markers = "extra == \"idna2008\""} - -[package.extras] -idna2008 = ["idna"] - [[package]] name = "rich" version = "12.6.0" @@ -1614,4 +1644,4 @@ watchmedo = ["PyYAML (>=3.10)"] [metadata] lock-version = "2.0" python-versions = "^3.10" -content-hash = "f704caa03e3c246b045eba40619de3c7c3f494cb17fddcff25a4304aa7950aa9" +content-hash = "977176f55a6f2c9bcff733738d40d2ad6230c4729e388a812d0564fcce9643f1" diff --git a/pyproject.toml b/pyproject.toml index 8fdf4b94b..f8d62e2b1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -37,6 +37,7 @@ pyhumps = "^3.7.3" cachetools = "^5.2.0" dictdiffer = "^0.9.0" python-schema-registry-client = "^2.4.1" +httpx = "^0.24.1" [tool.poetry.group.dev.dependencies] pytest = "^7.1.2" @@ -51,6 +52,8 @@ isort = "^5.12.0" typer-cli = "^0.0.13" pyright = "^1.1.314" pytest-rerunfailures = "^11.1.2" +pytest-asyncio = "^0.21.1" +pytest-httpx = "^0.22.0" [tool.poetry.group.docs] optional = true @@ -63,6 +66,11 @@ mkdocs-material-extensions = "^1.1.1" mkdocs-glightbox = "^0.3.1" mike = "^1.1.2" + +[tool.poetry.group.dependencies.dependencies] +aiofiles = "^23.1.0" +async-property = "^0.2.2" + [tool.poetry_bumpversion.file."kpops/__init__.py"] [build-system] diff --git a/tests/component_handlers/topic/test_proxy_wrapper.py b/tests/component_handlers/topic/test_proxy_wrapper.py index b3f823a36..30f940233 100644 --- a/tests/component_handlers/topic/test_proxy_wrapper.py +++ b/tests/component_handlers/topic/test_proxy_wrapper.py @@ -4,7 +4,9 @@ from unittest.mock import MagicMock, patch import pytest +import pytest_asyncio import responses +from pytest_httpx import HTTPXMock from pytest_mock import MockerFixture from kpops.cli.pipeline_config import PipelineConfig @@ -29,9 +31,8 @@ def log_info_mock(self, mocker: MockerFixture) -> MagicMock: def log_debug_mock(self, mocker: MockerFixture) -> MagicMock: return mocker.patch("kpops.component_handlers.topic.proxy_wrapper.log.debug") - @pytest.fixture(autouse=True) - @responses.activate - def setup(self): + @pytest_asyncio.fixture(autouse=True) + async def setup(self, httpx_mock: HTTPXMock): config = PipelineConfig( defaults_path=DEFAULTS_PATH, environment="development", kafka_rest_host=HOST ) @@ -42,16 +43,17 @@ def setup(self): ) as f: cluster_response = json.load(f) - responses.add( - responses.GET, - f"{HOST}/v3/clusters", + httpx_mock.add_response( + method="GET", + url=f"{HOST}/v3/clusters", json=cluster_response, - status=200, + status_code=200, ) assert self.proxy_wrapper.host == HOST - assert self.proxy_wrapper.cluster_id == "cluster-1" + assert await self.proxy_wrapper.cluster_id == "cluster-1" - def test_should_raise_exception_when_host_is_not_set(self): + @pytest.mark.asyncio + async def test_should_raise_exception_when_host_is_not_set(self): config = PipelineConfig(defaults_path=DEFAULTS_PATH, environment="development") config.kafka_rest_host = None with pytest.raises(ValueError) as exception: @@ -61,8 +63,9 @@ def test_should_raise_exception_when_host_is_not_set(self): == "The Kafka REST Proxy host is not set. Please set the host in the config.yaml using the kafka_rest_host property or set the environemt variable KPOPS_REST_PROXY_HOST." ) - @patch("requests.post") - def test_should_create_topic_with_all_topic_configuration( + @pytest.mark.asyncio + @patch("httpx.AsyncClient.post") + async def test_should_create_topic_with_all_topic_configuration( self, mock_post: MagicMock ): topic_spec = { @@ -76,7 +79,7 @@ def test_should_create_topic_with_all_topic_configuration( } with pytest.raises(KafkaRestProxyError): - self.proxy_wrapper.create_topic(topic_spec=TopicSpec(**topic_spec)) + await self.proxy_wrapper.create_topic(topic_spec=TopicSpec(**topic_spec)) mock_post.assert_called_with( url=f"{HOST}/v3/clusters/{self.proxy_wrapper.cluster_id}/topics", diff --git a/tests/components/test_kafka_app.py b/tests/components/test_kafka_app.py index 0ed1b6553..fc4e6ec93 100644 --- a/tests/components/test_kafka_app.py +++ b/tests/components/test_kafka_app.py @@ -57,7 +57,8 @@ def test_default_configs(self, config: PipelineConfig, handlers: ComponentHandle assert kafka_app.version == "2.9.0" assert kafka_app.namespace == "test-namespace" - def test_should_deploy_kafka_app( + @pytest.mark.asyncio + async def test_should_deploy_kafka_app( self, config: PipelineConfig, handlers: ComponentHandlers, mocker: MockerFixture ): kafka_app = KafkaApp( @@ -81,7 +82,7 @@ def test_should_deploy_kafka_app( ) mocker.patch.object(kafka_app, "get_helm_chart", return_value="test/test-chart") - kafka_app.deploy(dry_run=True) + await kafka_app.deploy(dry_run=True) print_helm_diff.assert_called_once() helm_upgrade_install.assert_called_once_with( diff --git a/tests/components/test_kafka_sink_connector.py b/tests/components/test_kafka_sink_connector.py index e95d6d3f0..061805965 100644 --- a/tests/components/test_kafka_sink_connector.py +++ b/tests/components/test_kafka_sink_connector.py @@ -154,7 +154,8 @@ def test_from_section_parsing_input_pattern( ) assert getattr(connector.app, "topics.regex") == topic_pattern - def test_deploy_order( + @pytest.mark.asyncio + async def test_deploy_order( self, connector: KafkaSinkConnector, mocker: MockerFixture, @@ -169,7 +170,7 @@ def test_deploy_order( mock = mocker.MagicMock() mock.attach_mock(mock_create_topics, "mock_create_topics") mock.attach_mock(mock_create_connector, "mock_create_connector") - connector.deploy(dry_run=True) + await connector.deploy(dry_run=True) assert mock.mock_calls == [ mocker.call.mock_create_topics(to_section=connector.to, dry_run=True), mocker.call.mock_create_connector( diff --git a/tests/components/test_kafka_source_connector.py b/tests/components/test_kafka_source_connector.py index 7f0c3779b..ce4f7b6b0 100644 --- a/tests/components/test_kafka_source_connector.py +++ b/tests/components/test_kafka_source_connector.py @@ -104,7 +104,8 @@ def test_from_section_raises_exception( ), ) - def test_deploy_order( + @pytest.mark.asyncio + async def test_deploy_order( self, connector: KafkaSourceConnector, mocker: MockerFixture, @@ -120,7 +121,7 @@ def test_deploy_order( mock = mocker.MagicMock() mock.attach_mock(mock_create_topics, "mock_create_topics") mock.attach_mock(mock_create_connector, "mock_create_connector") - connector.deploy(dry_run=True) + await connector.deploy(dry_run=True) assert mock.mock_calls == [ mocker.call.mock_create_topics(to_section=connector.to, dry_run=True), mocker.call.mock_create_connector( diff --git a/tests/components/test_kubernetes_app.py b/tests/components/test_kubernetes_app.py index 287608c81..81daee61f 100644 --- a/tests/components/test_kubernetes_app.py +++ b/tests/components/test_kubernetes_app.py @@ -56,7 +56,8 @@ def log_info_mock(self, mocker: MockerFixture) -> MagicMock: def app_value(self) -> KubernetesTestValue: return KubernetesTestValue(**{"name_override": "test-value"}) - def test_should_lazy_load_helm_wrapper_and_not_repo_add( + @pytest.mark.asyncio + async def test_should_lazy_load_helm_wrapper_and_not_repo_add( self, config: PipelineConfig, handlers: ComponentHandlers, @@ -76,7 +77,7 @@ def test_should_lazy_load_helm_wrapper_and_not_repo_add( kubernetes_app, "get_helm_chart", return_value="test/test-chart" ) - kubernetes_app.deploy(False) + await kubernetes_app.deploy(False) helm_mock.add_repo.assert_not_called() @@ -89,7 +90,8 @@ def test_should_lazy_load_helm_wrapper_and_not_repo_add( HelmUpgradeInstallFlags(), ) - def test_should_lazy_load_helm_wrapper_and_call_repo_add_when_implemented( + @pytest.mark.asyncio + async def test_should_lazy_load_helm_wrapper_and_call_repo_add_when_implemented( self, config: PipelineConfig, handlers: ComponentHandlers, @@ -115,7 +117,7 @@ def test_should_lazy_load_helm_wrapper_and_call_repo_add_when_implemented( new_callable=mocker.PropertyMock, ) - kubernetes_app.deploy(dry_run=False) + await kubernetes_app.deploy(dry_run=False) assert helm_mock.mock_calls == [ mocker.call.add_repo( @@ -133,7 +135,8 @@ def test_should_lazy_load_helm_wrapper_and_call_repo_add_when_implemented( ), ] - def test_should_raise_not_implemented_error_when_helm_chart_is_not_set( + @pytest.mark.asyncio + async def test_should_raise_not_implemented_error_when_helm_chart_is_not_set( self, config: PipelineConfig, handlers: ComponentHandlers, @@ -148,7 +151,7 @@ def test_should_raise_not_implemented_error_when_helm_chart_is_not_set( ) with pytest.raises(NotImplementedError) as error: - kubernetes_app.deploy(True) + await kubernetes_app.deploy(True) assert ( "Please implement the get_helm_chart() method of the kpops.components.base_components.kubernetes_app module." == str(error.value) diff --git a/tests/components/test_producer_app.py b/tests/components/test_producer_app.py index c9e96290a..72f136b1d 100644 --- a/tests/components/test_producer_app.py +++ b/tests/components/test_producer_app.py @@ -99,7 +99,8 @@ def test_output_topics(self, config: PipelineConfig, handlers: ComponentHandlers "first-extra-topic": "extra-topic-1" } - def test_deploy_order_when_dry_run_is_false( + @pytest.mark.asyncio + async def test_deploy_order_when_dry_run_is_false( self, producer_app: ProducerApp, mocker: MockerFixture, @@ -116,7 +117,7 @@ def test_deploy_order_when_dry_run_is_false( mock.attach_mock(mock_create_topics, "mock_create_topics") mock.attach_mock(mock_helm_upgrade_install, "mock_helm_upgrade_install") - producer_app.deploy(dry_run=False) + await producer_app.deploy(dry_run=False) assert mock.mock_calls == [ mocker.call.mock_create_topics(to_section=producer_app.to, dry_run=False), mocker.call.mock_helm_upgrade_install( diff --git a/tests/components/test_streams_app.py b/tests/components/test_streams_app.py index 77c75cb28..c71f2922f 100644 --- a/tests/components/test_streams_app.py +++ b/tests/components/test_streams_app.py @@ -256,7 +256,8 @@ def test_weave_inputs_from_prev_component( assert streams_app.app.streams.input_topics == ["prev-output-topic", "b", "a"] - def test_deploy_order_when_dry_run_is_false( + @pytest.mark.asyncio + async def test_deploy_order_when_dry_run_is_false( self, config: PipelineConfig, handlers: ComponentHandlers, @@ -305,7 +306,7 @@ def test_deploy_order_when_dry_run_is_false( mock.attach_mock(mock_helm_upgrade_install, "mock_helm_upgrade_install") dry_run = False - streams_app.deploy(dry_run=dry_run) + await streams_app.deploy(dry_run=dry_run) assert mock.mock_calls == [ mocker.call.mock_create_topics(to_section=streams_app.to, dry_run=dry_run), From 10607d4d9a2e8f5d8a6355703ca42fd3e0bfe86b Mon Sep 17 00:00:00 2001 From: Alejandro Jaramillo Date: Mon, 24 Jul 2023 14:34:54 +0200 Subject: [PATCH 02/23] Migrate test for proxy wrapper --- .../topic/test_proxy_wrapper.py | 118 ++++++++++-------- 1 file changed, 65 insertions(+), 53 deletions(-) diff --git a/tests/component_handlers/topic/test_proxy_wrapper.py b/tests/component_handlers/topic/test_proxy_wrapper.py index 30f940233..3224f0f03 100644 --- a/tests/component_handlers/topic/test_proxy_wrapper.py +++ b/tests/component_handlers/topic/test_proxy_wrapper.py @@ -5,7 +5,6 @@ import pytest import pytest_asyncio -import responses from pytest_httpx import HTTPXMock from pytest_mock import MockerFixture @@ -87,12 +86,15 @@ async def test_should_create_topic_with_all_topic_configuration( json=topic_spec, ) - @patch("requests.post") - def test_should_create_topic_with_no_configuration(self, mock_post: MagicMock): + @pytest.mark.asyncio + @patch("httpx.AsyncClient.post") + async def test_should_create_topic_with_no_configuration( + self, mock_post: MagicMock + ): topic_spec: dict[str, Any] = {"topic_name": "topic-X"} with pytest.raises(KafkaRestProxyError): - self.proxy_wrapper.create_topic(topic_spec=TopicSpec(**topic_spec)) + await self.proxy_wrapper.create_topic(topic_spec=TopicSpec(**topic_spec)) mock_post.assert_called_with( url=f"{HOST}/v3/clusters/{self.proxy_wrapper.cluster_id}/topics", @@ -100,24 +102,26 @@ def test_should_create_topic_with_no_configuration(self, mock_post: MagicMock): json=topic_spec, ) - @patch("requests.get") - def test_should_call_get_topic(self, mock_get: MagicMock): + @pytest.mark.asyncio + @patch("httpx.AsyncClient.get") + async def test_should_call_get_topic(self, mock_get: MagicMock): topic_name = "topic-X" with pytest.raises(KafkaRestProxyError): - self.proxy_wrapper.get_topic(topic_name=topic_name) + await self.proxy_wrapper.get_topic(topic_name=topic_name) mock_get.assert_called_with( url=f"{HOST}/v3/clusters/{self.proxy_wrapper.cluster_id}/topics/{topic_name}", headers=HEADERS, ) - @patch("requests.post") - def test_should_call_batch_alter_topic_config(self, mock_put: MagicMock): + @pytest.mark.asyncio + @patch("httpx.AsyncClient.post") + async def test_should_call_batch_alter_topic_config(self, mock_put: MagicMock): topic_name = "topic-X" with pytest.raises(KafkaRestProxyError): - self.proxy_wrapper.batch_alter_topic_config( + await self.proxy_wrapper.batch_alter_topic_config( topic_name=topic_name, json_body=[ {"name": "cleanup.policy", "operation": "DELETE"}, @@ -136,30 +140,34 @@ def test_should_call_batch_alter_topic_config(self, mock_put: MagicMock): }, ) - @patch("requests.delete") - def test_should_call_delete_topic(self, mock_delete: MagicMock): + @pytest.mark.asyncio + @patch("httpx.AsyncClient.delete") + async def test_should_call_delete_topic(self, mock_delete: MagicMock): topic_name = "topic-X" with pytest.raises(KafkaRestProxyError): - self.proxy_wrapper.delete_topic(topic_name=topic_name) + await self.proxy_wrapper.delete_topic(topic_name=topic_name) mock_delete.assert_called_with( url=f"{HOST}/v3/clusters/{self.proxy_wrapper.cluster_id}/topics/{topic_name}", headers=HEADERS, ) - @patch("requests.get") - def test_should_call_get_broker_config(self, mock_get: MagicMock): + @pytest.mark.asyncio + @patch("httpx.AsyncClient.get") + async def test_should_call_get_broker_config(self, mock_get: MagicMock): with pytest.raises(KafkaRestProxyError): - self.proxy_wrapper.get_broker_config() + await self.proxy_wrapper.get_broker_config() mock_get.assert_called_with( url=f"{HOST}/v3/clusters/{self.proxy_wrapper.cluster_id}/brokers/-/configs", headers=HEADERS, ) - @responses.activate - def test_should_log_topic_creation(self, log_info_mock: MagicMock): + @pytest.mark.asyncio + async def test_should_log_topic_creation( + self, log_info_mock: MagicMock, httpx_mock: HTTPXMock + ): topic_spec = { "topic_name": "topic-X", "partitions_count": 1, @@ -170,31 +178,35 @@ def test_should_log_topic_creation(self, log_info_mock: MagicMock): ], } - responses.add( - responses.POST, - f"{HOST}/v3/clusters/cluster-1/topics", + httpx_mock.add_response( + method="POST", + url=f"{HOST}/v3/clusters/cluster-1/topics", json=topic_spec, headers=HEADERS, - status=201, + status_code=201, ) - self.proxy_wrapper.create_topic(topic_spec=TopicSpec(**topic_spec)) + await self.proxy_wrapper.create_topic(topic_spec=TopicSpec(**topic_spec)) log_info_mock.assert_called_once_with("Topic topic-X created.") - @responses.activate - def test_should_log_topic_deletion(self, log_info_mock: MagicMock): + @pytest.mark.asyncio + async def test_should_log_topic_deletion( + self, log_info_mock: MagicMock, httpx_mock: HTTPXMock + ): topic_name = "topic-X" - responses.add( - responses.DELETE, - f"{HOST}/v3/clusters/cluster-1/topics/{topic_name}", + httpx_mock.add_response( + method="DELETE", + url=f"{HOST}/v3/clusters/cluster-1/topics/{topic_name}", headers=HEADERS, - status=204, + status_code=204, ) - self.proxy_wrapper.delete_topic(topic_name=topic_name) + await self.proxy_wrapper.delete_topic(topic_name=topic_name) log_info_mock.assert_called_once_with("Topic topic-X deleted.") - @responses.activate - def test_should_get_topic(self, log_debug_mock: MagicMock): + @pytest.mark.asyncio + async def test_should_get_topic( + self, log_debug_mock: MagicMock, httpx_mock: HTTPXMock + ): res = { "kind": "KafkaTopic", "metadata": { @@ -214,55 +226,55 @@ def test_should_get_topic(self, log_debug_mock: MagicMock): topic_name = "topic-X" - responses.add( - responses.GET, - f"{HOST}/v3/clusters/cluster-1/topics/{topic_name}", + httpx_mock.add_response( + method="GET", + url=f"{HOST}/v3/clusters/cluster-1/topics/{topic_name}", headers=HEADERS, - status=200, + status_code=200, json=res, ) - get_topic_response = self.proxy_wrapper.get_topic(topic_name=topic_name) + get_topic_response = await self.proxy_wrapper.get_topic(topic_name=topic_name) log_debug_mock.assert_any_call("Topic topic-X found.") assert get_topic_response == topic_response - @responses.activate - def test_should_rais_topic_not_found_exception_get_topic( - self, log_debug_mock: MagicMock + @pytest.mark.asyncio + async def test_should_rais_topic_not_found_exception_get_topic( + self, log_debug_mock: MagicMock, httpx_mock: HTTPXMock ): topic_name = "topic-X" - responses.add( - responses.GET, - f"{HOST}/v3/clusters/cluster-1/topics/{topic_name}", + httpx_mock.add_response( + method="GET", + url=f"{HOST}/v3/clusters/cluster-1/topics/{topic_name}", headers=HEADERS, - status=404, + status_code=404, json={ "error_code": 40403, "message": "This server does not host this topic-partition.", }, ) with pytest.raises(TopicNotFoundException): - self.proxy_wrapper.get_topic(topic_name=topic_name) + await self.proxy_wrapper.get_topic(topic_name=topic_name) log_debug_mock.assert_any_call("Topic topic-X not found.") - @responses.activate - def test_should_log_reset_default_topic_config_when_deleted( - self, log_info_mock: MagicMock + @pytest.mark.asyncio + async def test_should_log_reset_default_topic_config_when_deleted( + self, log_info_mock: MagicMock, httpx_mock: HTTPXMock ): topic_name = "topic-X" config_name = "cleanup.policy" - responses.add( - responses.POST, - f"{HOST}/v3/clusters/cluster-1/topics/{topic_name}/configs:alter", + httpx_mock.add_response( + method="POST", + url=f"{HOST}/v3/clusters/cluster-1/topics/{topic_name}/configs:alter", headers=HEADERS, json={"data": [{"name": config_name, "operation": "DELETE"}]}, - status=204, + status_code=204, ) - self.proxy_wrapper.batch_alter_topic_config( + await self.proxy_wrapper.batch_alter_topic_config( topic_name=topic_name, json_body=[{"name": config_name, "operation": "DELETE"}], ) From 7562d9e21f1f2d340fa20adf74093376c34f8b29 Mon Sep 17 00:00:00 2001 From: Alejandro Jaramillo Date: Mon, 24 Jul 2023 14:39:59 +0200 Subject: [PATCH 03/23] Replace response type --- kpops/component_handlers/utils/exception.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kpops/component_handlers/utils/exception.py b/kpops/component_handlers/utils/exception.py index 340bc42b4..89ed9d84b 100644 --- a/kpops/component_handlers/utils/exception.py +++ b/kpops/component_handlers/utils/exception.py @@ -1,12 +1,12 @@ import logging -import requests +import httpx log = logging.getLogger("RequestException") class RequestsException(Exception): - def __init__(self, response: requests.Response) -> None: + def __init__(self, response: httpx.Response) -> None: self.error_code = response.status_code self.error_msg = "Something went wrong!" try: @@ -14,7 +14,7 @@ def __init__(self, response: requests.Response) -> None: f"The request responded with the code {self.error_code}. Error body: {response.json()}" ) response.raise_for_status() - except requests.HTTPError as e: + except httpx.HTTPError as e: self.error_msg = str(e) log.error(f"More information: {self.error_msg}") super().__init__() From cea7d743c4b4804e2db58bdddbc9bb6d4c034d8a Mon Sep 17 00:00:00 2001 From: Alejandro Jaramillo Date: Mon, 24 Jul 2023 14:55:37 +0200 Subject: [PATCH 04/23] Migrate topic handler to asyncio --- kpops/component_handlers/topic/handler.py | 32 +++--- .../topic/test_topic_handler.py | 101 +++++++++++------- 2 files changed, 76 insertions(+), 57 deletions(-) diff --git a/kpops/component_handlers/topic/handler.py b/kpops/component_handlers/topic/handler.py index 1df0d106a..9e4d7abab 100644 --- a/kpops/component_handlers/topic/handler.py +++ b/kpops/component_handlers/topic/handler.py @@ -26,15 +26,15 @@ class TopicHandler: def __init__(self, proxy_wrapper: ProxyWrapper): self.proxy_wrapper = proxy_wrapper - def create_topics(self, to_section: ToSection, dry_run: bool) -> None: + async def create_topics(self, to_section: ToSection, dry_run: bool) -> None: for topic_name, topic_config in to_section.topics.items(): topic_spec = self.__prepare_body(topic_name, topic_config) if dry_run: - self.__dry_run_topic_creation(topic_name, topic_spec, topic_config) + await self.__dry_run_topic_creation(topic_name, topic_spec, topic_config) else: try: - self.proxy_wrapper.get_topic(topic_name=topic_name) - topic_config_in_cluster = self.proxy_wrapper.get_topic_config( + await self.proxy_wrapper.get_topic(topic_name=topic_name) + topic_config_in_cluster = await self.proxy_wrapper.get_topic_config( topic_name=topic_name ) differences = self.__get_topic_config_diff( @@ -52,7 +52,7 @@ def create_topics(self, to_section: ToSection, dry_run: bool) -> None: json_body.append( {"name": difference.key, "value": config_value} ) - self.proxy_wrapper.batch_alter_topic_config( + await self.proxy_wrapper.batch_alter_topic_config( topic_name=topic_name, json_body=json_body, ) @@ -62,16 +62,16 @@ def create_topics(self, to_section: ToSection, dry_run: bool) -> None: f"Topic Creation: config of topic {topic_name} didn't change. Skipping update." ) except TopicNotFoundException: - self.proxy_wrapper.create_topic(topic_spec=topic_spec) + await self.proxy_wrapper.create_topic(topic_spec=topic_spec) - def delete_topics(self, to_section: ToSection, dry_run: bool) -> None: + async def delete_topics(self, to_section: ToSection, dry_run: bool) -> None: for topic_name in to_section.topics.keys(): if dry_run: - self.__dry_run_topic_deletion(topic_name=topic_name) + await self.__dry_run_topic_deletion(topic_name=topic_name) else: try: - self.proxy_wrapper.get_topic(topic_name=topic_name) - self.proxy_wrapper.delete_topic(topic_name=topic_name) + await self.proxy_wrapper.get_topic(topic_name=topic_name) + await self.proxy_wrapper.delete_topic(topic_name=topic_name) except TopicNotFoundException: log.warning( f"Topic Deletion: topic {topic_name} does not exist in the cluster and cannot be deleted. Skipping." @@ -86,17 +86,17 @@ def __get_topic_config_diff( ) return list(Diff.from_dicts(comparable_in_cluster_config_dict, current_config)) - def __dry_run_topic_creation( + async def __dry_run_topic_creation( self, topic_name: str, topic_spec: TopicSpec, topic_config: TopicConfig | None = None, ) -> None: try: - topic_in_cluster = self.proxy_wrapper.get_topic(topic_name=topic_name) + topic_in_cluster = await self.proxy_wrapper.get_topic(topic_name=topic_name) topic_name = topic_in_cluster.topic_name if topic_config: - topic_config_in_cluster = self.proxy_wrapper.get_topic_config( + topic_config_in_cluster = await self.proxy_wrapper.get_topic_config( topic_name=topic_name ) in_cluster_config, new_config = parse_and_compare_topic_configs( @@ -115,7 +115,7 @@ def __dry_run_topic_creation( } log.debug(error_message) - broker_config = self.proxy_wrapper.get_broker_config() + broker_config = await self.proxy_wrapper.get_broker_config() effective_config = get_effective_config(broker_config) self.__check_partition_count(topic_in_cluster, topic_spec, effective_config) @@ -172,9 +172,9 @@ def __check_replication_factor( f"Topic Creation: replication factor of topic {topic_name} changed! Replication factor of topic {topic_name} is {replication_factor}. The given replication count {topic_spec.replication_factor}." ) - def __dry_run_topic_deletion(self, topic_name: str) -> None: + async def __dry_run_topic_deletion(self, topic_name: str) -> None: try: - topic_in_cluster = self.proxy_wrapper.get_topic(topic_name=topic_name) + topic_in_cluster = await self.proxy_wrapper.get_topic(topic_name=topic_name) log.info( magentaify( f"Topic Deletion: topic {topic_in_cluster.topic_name} exists in the cluster. Deleting topic." diff --git a/tests/component_handlers/topic/test_topic_handler.py b/tests/component_handlers/topic/test_topic_handler.py index c53a7a60d..63515b0aa 100644 --- a/tests/component_handlers/topic/test_topic_handler.py +++ b/tests/component_handlers/topic/test_topic_handler.py @@ -2,9 +2,10 @@ import logging from pathlib import Path from unittest import mock -from unittest.mock import MagicMock +from unittest.mock import MagicMock, AsyncMock import pytest +import pytest_asyncio from pytest_mock import MockerFixture from kpops.component_handlers.topic.exception import ( @@ -49,8 +50,8 @@ def log_warning_mock(self, mocker: MockerFixture) -> MagicMock: def log_error_mock(self, mocker: MockerFixture) -> MagicMock: return mocker.patch("kpops.component_handlers.topic.handler.log.error") - @pytest.fixture(autouse=True) - def get_topic_response_mock(self) -> MagicMock: + @pytest_asyncio.fixture(autouse=True) + async def get_topic_response_mock(self) -> MagicMock: with open( DEFAULTS_PATH / "kafka_rest_proxy_responses/get_topic_response.json" ) as f: @@ -66,7 +67,7 @@ def get_topic_response_mock(self) -> MagicMock: ) as f: response_topic_config = json.load(f) - wrapper = MagicMock() + wrapper = AsyncMock() wrapper.get_topic.return_value = TopicResponse(**response) wrapper.get_broker_config.return_value = BrokerConfigResponse(**broker_response) wrapper.get_topic_config.return_value = TopicConfigResponse( @@ -74,8 +75,8 @@ def get_topic_response_mock(self) -> MagicMock: ) return wrapper - @pytest.fixture(autouse=True) - def get_default_topic_response_mock(self) -> MagicMock: + @pytest_asyncio.fixture(autouse=True) + async def get_default_topic_response_mock(self) -> MagicMock: with open( DEFAULTS_PATH / "kafka_rest_proxy_responses/get_default_topic_response.json" ) as f: @@ -86,13 +87,14 @@ def get_default_topic_response_mock(self) -> MagicMock: ) as f: broker_response = json.load(f) - wrapper = MagicMock() + wrapper = AsyncMock() wrapper.get_topic.return_value = TopicResponse(**response) wrapper.get_broker_config.return_value = BrokerConfigResponse(**broker_response) return wrapper - def test_should_call_create_topic_with_dry_run_false(self): - wrapper = MagicMock() + @pytest.mark.asyncio + async def test_should_call_create_topic_with_dry_run_false(self): + wrapper = AsyncMock() wrapper.get_topic.side_effect = TopicNotFoundException() topic_handler = TopicHandler(proxy_wrapper=wrapper) @@ -104,7 +106,7 @@ def test_should_call_create_topic_with_dry_run_false(self): ) to_section = ToSection(topics={TopicName("topic-X"): topic_config}) - topic_handler.create_topics(to_section=to_section, dry_run=False) + await topic_handler.create_topics(to_section=to_section, dry_run=False) topic_spec = { "topic_name": "topic-X", @@ -119,7 +121,8 @@ def test_should_call_create_topic_with_dry_run_false(self): wrapper.create_topic.assert_called_once_with(topic_spec=TopicSpec(**topic_spec)) wrapper.__dry_run_topic_creation.assert_not_called() - def test_should_call_update_topic_config_when_topic_exists_and_with_dry_run_false( + @pytest.mark.asyncio + async def test_should_call_update_topic_config_when_topic_exists_and_with_dry_run_false( self, get_topic_response_mock: MagicMock ): wrapper = get_topic_response_mock @@ -133,7 +136,7 @@ def test_should_call_update_topic_config_when_topic_exists_and_with_dry_run_fals ) to_section = ToSection(topics={TopicName("topic-X"): topic_config}) - topic_handler.create_topics(to_section=to_section, dry_run=False) + await topic_handler.create_topics(to_section=to_section, dry_run=False) wrapper.batch_alter_topic_config.assert_called_once_with( topic_name="topic-X", @@ -145,7 +148,8 @@ def test_should_call_update_topic_config_when_topic_exists_and_with_dry_run_fals ) wrapper.__dry_run_topic_creation.assert_not_called() - def test_should_update_topic_config_when_one_config_changed( + @pytest.mark.asyncio + async def test_should_update_topic_config_when_one_config_changed( self, log_info_mock: MagicMock, get_topic_response_mock: MagicMock ): wrapper = get_topic_response_mock @@ -160,14 +164,15 @@ def test_should_update_topic_config_when_one_config_changed( ) to_section = ToSection(topics={TopicName("topic-X"): topic_config}) - topic_handler.create_topics(to_section=to_section, dry_run=False) + await topic_handler.create_topics(to_section=to_section, dry_run=False) wrapper.batch_alter_topic_config.assert_called_once_with( topic_name="topic-X", json_body=[{"name": "cleanup.policy", "value": "delete"}], ) - def test_should_not_update_topic_config_when_config_not_changed( + @pytest.mark.asyncio + async def test_should_not_update_topic_config_when_config_not_changed( self, log_info_mock: MagicMock, get_topic_response_mock: MagicMock ): wrapper = get_topic_response_mock @@ -182,14 +187,15 @@ def test_should_not_update_topic_config_when_config_not_changed( ) to_section = ToSection(topics={TopicName("topic-X"): topic_config}) - topic_handler.create_topics(to_section=to_section, dry_run=False) + await topic_handler.create_topics(to_section=to_section, dry_run=False) wrapper.batch_alter_topic_config.assert_not_called() log_info_mock.assert_called_once_with( "Topic Creation: config of topic topic-X didn't change. Skipping update." ) - def test_should_not_update_topic_config_when_config_not_changed_and_not_ordered( + @pytest.mark.asyncio + async def test_should_not_update_topic_config_when_config_not_changed_and_not_ordered( self, log_info_mock: MagicMock, get_topic_response_mock: MagicMock ): wrapper = get_topic_response_mock @@ -203,14 +209,15 @@ def test_should_not_update_topic_config_when_config_not_changed_and_not_ordered( ) to_section = ToSection(topics={TopicName("topic-X"): topic_config}) - topic_handler.create_topics(to_section=to_section, dry_run=False) + await topic_handler.create_topics(to_section=to_section, dry_run=False) wrapper.batch_alter_topic_config.assert_not_called() log_info_mock.assert_called_once_with( "Topic Creation: config of topic topic-X didn't change. Skipping update." ) - def test_should_call_reset_topic_config_when_topic_exists_dry_run_false_and_topic_configs_change( + @pytest.mark.asyncio + async def test_should_call_reset_topic_config_when_topic_exists_dry_run_false_and_topic_configs_change( self, get_topic_response_mock: MagicMock ): wrapper = get_topic_response_mock @@ -225,7 +232,7 @@ def test_should_call_reset_topic_config_when_topic_exists_dry_run_false_and_topi ) to_section = ToSection(topics={TopicName("topic-X"): topic_config}) - topic_handler.create_topics(to_section=to_section, dry_run=False) + await topic_handler.create_topics(to_section=to_section, dry_run=False) wrapper.batch_alter_topic_config.assert_called_once_with( topic_name="topic-X", @@ -233,7 +240,10 @@ def test_should_call_reset_topic_config_when_topic_exists_dry_run_false_and_topi ) wrapper.__dry_run_topic_creation.assert_not_called() - def test_should_not_call_create_topics_with_dry_run_true_and_topic_not_exists(self): + @pytest.mark.asyncio + async def test_should_not_call_create_topics_with_dry_run_true_and_topic_not_exists( + self, + ): wrapper = MagicMock() wrapper.get_topic.side_effect = TopicNotFoundException() topic_handler = TopicHandler(proxy_wrapper=wrapper) @@ -246,11 +256,12 @@ def test_should_not_call_create_topics_with_dry_run_true_and_topic_not_exists(se ) to_section = ToSection(topics={TopicName("topic-X"): topic_config}) - topic_handler.create_topics(to_section=to_section, dry_run=True) + await topic_handler.create_topics(to_section=to_section, dry_run=True) wrapper.create_topic.assert_not_called() - def test_should_print_message_with_dry_run_true_and_topic_not_exists( + @pytest.mark.asyncio + async def test_should_print_message_with_dry_run_true_and_topic_not_exists( self, log_info_mock: MagicMock ): wrapper = MagicMock() @@ -267,7 +278,7 @@ def test_should_print_message_with_dry_run_true_and_topic_not_exists( ) to_section = ToSection(topics={TopicName("topic-X"): topic_config}) - topic_handler.create_topics(to_section=to_section, dry_run=True) + await topic_handler.create_topics(to_section=to_section, dry_run=True) log_info_mock.assert_called_once_with( greenify( @@ -275,7 +286,8 @@ def test_should_print_message_with_dry_run_true_and_topic_not_exists( ) ) - def test_should_print_message_if_dry_run_and_topic_exists_with_same_partition_count_and_replication_factor( + @pytest.mark.asyncio + async def test_should_print_message_if_dry_run_and_topic_exists_with_same_partition_count_and_replication_factor( self, log_info_mock: MagicMock, log_debug_mock: MagicMock, @@ -292,7 +304,7 @@ def test_should_print_message_if_dry_run_and_topic_exists_with_same_partition_co ) to_section = ToSection(topics={TopicName("topic-X"): topic_config}) - topic_handler.create_topics(to_section=to_section, dry_run=True) + await topic_handler.create_topics(to_section=to_section, dry_run=True) wrapper.get_topic_config.assert_called_once() # dry run requests the config to create the diff assert log_info_mock.mock_calls == [ mock.call("Topic Creation: topic-X already exists in cluster.") @@ -311,7 +323,8 @@ def test_should_print_message_if_dry_run_and_topic_exists_with_same_partition_co ), ] - def test_should_print_message_if_dry_run_and_topic_exists_with_default_partition_count_and_replication_factor( + @pytest.mark.asyncio + async def test_should_print_message_if_dry_run_and_topic_exists_with_default_partition_count_and_replication_factor( self, log_info_mock: MagicMock, log_debug_mock: MagicMock, @@ -326,7 +339,7 @@ def test_should_print_message_if_dry_run_and_topic_exists_with_default_partition ) to_section = ToSection(topics={TopicName("topic-X"): topic_config}) - topic_handler.create_topics(to_section=to_section, dry_run=True) + await topic_handler.create_topics(to_section=to_section, dry_run=True) wrapper.get_topic_config.assert_called_once() # dry run requests the config to create the diff assert log_info_mock.mock_calls == [ mock.call("Config changes for topic topic-X:"), @@ -349,7 +362,8 @@ def test_should_print_message_if_dry_run_and_topic_exists_with_default_partition ), ] - def test_should_exit_if_dry_run_and_topic_exists_different_partition_count( + @pytest.mark.asyncio + async def test_should_exit_if_dry_run_and_topic_exists_different_partition_count( self, get_topic_response_mock: MagicMock ): wrapper = get_topic_response_mock @@ -368,10 +382,11 @@ def test_should_exit_if_dry_run_and_topic_exists_different_partition_count( TopicTransactionError, match="Topic Creation: partition count of topic topic-X changed! Partitions count of topic topic-X is 10. The given partitions count 200.", ): - topic_handler.create_topics(to_section=to_section, dry_run=True) + await topic_handler.create_topics(to_section=to_section, dry_run=True) wrapper.get_topic_config.assert_called_once() # dry run requests the config to create the diff - def test_should_exit_if_dry_run_and_topic_exists_different_replication_factor( + @pytest.mark.asyncio + async def test_should_exit_if_dry_run_and_topic_exists_different_replication_factor( self, get_topic_response_mock: MagicMock ): wrapper = get_topic_response_mock @@ -390,10 +405,11 @@ def test_should_exit_if_dry_run_and_topic_exists_different_replication_factor( TopicTransactionError, match="Topic Creation: replication factor of topic topic-X changed! Replication factor of topic topic-X is 3. The given replication count 300.", ): - topic_handler.create_topics(to_section=to_section, dry_run=True) + await topic_handler.create_topics(to_section=to_section, dry_run=True) wrapper.get_topic_config.assert_called_once() # dry run requests the config to create the diff - def test_should_log_correct_message_when_delete_existing_topic_dry_run( + @pytest.mark.asyncio + async def test_should_log_correct_message_when_delete_existing_topic_dry_run( self, log_info_mock: MagicMock, get_topic_response_mock: MagicMock ): wrapper = get_topic_response_mock @@ -408,7 +424,7 @@ def test_should_log_correct_message_when_delete_existing_topic_dry_run( ) to_section = ToSection(topics={TopicName("topic-X"): topic_config}) - topic_handler.delete_topics(to_section, True) + await topic_handler.delete_topics(to_section, True) wrapper.get_topic.assert_called_once_with(topic_name="topic-X") log_info_mock.assert_called_once_with( @@ -417,7 +433,8 @@ def test_should_log_correct_message_when_delete_existing_topic_dry_run( ) ) - def test_should_log_correct_message_when_delete_non_existing_topic_dry_run( + @pytest.mark.asyncio + async def test_should_log_correct_message_when_delete_non_existing_topic_dry_run( self, log_warning_mock: MagicMock ): wrapper = MagicMock() @@ -433,15 +450,16 @@ def test_should_log_correct_message_when_delete_non_existing_topic_dry_run( ) to_section = ToSection(topics={TopicName("topic-X"): topic_config}) - topic_handler.delete_topics(to_section, True) + await topic_handler.delete_topics(to_section, True) wrapper.get_topic.assert_called_once_with(topic_name="topic-X") log_warning_mock.assert_called_once_with( "Topic Deletion: topic topic-X does not exist in the cluster and cannot be deleted. Skipping." ) - def test_should_call_delete_topic_not_dry_run(self): - wrapper = MagicMock() + @pytest.mark.asyncio + async def test_should_call_delete_topic_not_dry_run(self): + wrapper = AsyncMock() topic_handler = TopicHandler(proxy_wrapper=wrapper) topic_config = TopicConfig( @@ -452,14 +470,15 @@ def test_should_call_delete_topic_not_dry_run(self): ) to_section = ToSection(topics={TopicName("topic-X"): topic_config}) - topic_handler.delete_topics(to_section, False) + await topic_handler.delete_topics(to_section, False) assert wrapper.mock_calls == [ mock.call.get_topic(topic_name="topic-X"), mock.call.delete_topic(topic_name="topic-X"), ] - def test_should_print_correct_warning_when_deleting_topic_that_does_not_exists_not_dry_run( + @pytest.mark.asyncio + async def test_should_print_correct_warning_when_deleting_topic_that_does_not_exists_not_dry_run( self, log_warning_mock: MagicMock ): wrapper = MagicMock() @@ -474,7 +493,7 @@ def test_should_print_correct_warning_when_deleting_topic_that_does_not_exists_n configs={"cleanup.policy": "compact", "compression.type": "gzip"}, ) to_section = ToSection(topics={TopicName("topic-X"): topic_config}) - topic_handler.delete_topics(to_section, False) + await topic_handler.delete_topics(to_section, False) wrapper.get_topic.assert_called_once_with(topic_name="topic-X") log_warning_mock.assert_called_once_with( From 53bcfcd6878aa76765ba4c7c7fb89fbe9ac48882 Mon Sep 17 00:00:00 2001 From: Alejandro Jaramillo Date: Mon, 24 Jul 2023 16:59:04 +0200 Subject: [PATCH 05/23] Save progress --- kpops/cli/main.py | 2 +- .../kafka_connect/connect_wrapper.py | 52 ++-- .../kafka_connect/kafka_connect_handler.py | 62 ++-- .../kafka_connect/timeout.py | 18 +- kpops/component_handlers/topic/handler.py | 4 +- kpops/components/base_components/kafka_app.py | 2 +- .../base_components/kafka_connector.py | 4 +- .../kafka_connect/test_connect_handler.py | 72 +++-- .../kafka_connect/test_connect_wrapper.py | 284 ++++++++++-------- .../topic/test_topic_handler.py | 2 +- 10 files changed, 285 insertions(+), 217 deletions(-) diff --git a/kpops/cli/main.py b/kpops/cli/main.py index 00be4b5f6..306b28ab3 100644 --- a/kpops/cli/main.py +++ b/kpops/cli/main.py @@ -280,7 +280,7 @@ def deploy( async def async_deploy(): for component in steps_to_apply: log_action("Deploy", component) - component.deploy(dry_run) + await component.deploy(dry_run) asyncio.run(async_deploy()) diff --git a/kpops/component_handlers/kafka_connect/connect_wrapper.py b/kpops/component_handlers/kafka_connect/connect_wrapper.py index 57f7c2653..e83568a85 100644 --- a/kpops/component_handlers/kafka_connect/connect_wrapper.py +++ b/kpops/component_handlers/kafka_connect/connect_wrapper.py @@ -1,8 +1,8 @@ +import asyncio import logging -import time -from time import sleep from typing import Any +import httpx import requests from kpops.component_handlers.kafka_connect.exception import ( @@ -38,7 +38,7 @@ def __init__(self, host: str | None): def host(self) -> str: return self._host - def create_connector( + async def create_connector( self, connector_name: str, kafka_connect_config: KafkaConnectConfig ) -> KafkaConnectResponse: """ @@ -51,9 +51,12 @@ def create_connector( config_json = kafka_connect_config.dict(exclude_none=True) connect_data = {"name": connector_name, "config": config_json} - response = requests.post( + client = httpx.AsyncClient() + response = await client.post( url=f"{self._host}/connectors", headers=HEADERS, json=connect_data ) + await client.aclose() + if response.status_code == requests.status_codes.codes.created: log.info(f"Connector {connector_name} created.") log.debug(response.json()) @@ -62,20 +65,22 @@ def create_connector( log.warning( "Rebalancing in progress while creating a connector... Retrying..." ) - time.sleep(1) - self.create_connector(connector_name, kafka_connect_config) + await asyncio.sleep(1) + await self.create_connector(connector_name, kafka_connect_config) raise KafkaConnectError(response) - def get_connector(self, connector_name: str) -> KafkaConnectResponse: + async def get_connector(self, connector_name: str) -> KafkaConnectResponse: """ Get information about the connector. API Reference: https://docs.confluent.io/platform/current/connect/references/restapi.html#get--connectors-(string-name) :param connector_name: Nameof the crated connector :return: Information about the connector """ - response = requests.get( + client = httpx.AsyncClient() + response = await client.get( url=f"{self._host}/connectors/{connector_name}", headers=HEADERS ) + await client.aclose() if response.status_code == requests.status_codes.codes.ok: log.info(f"Connector {connector_name} exists.") log.debug(response.json()) @@ -87,11 +92,11 @@ def get_connector(self, connector_name: str) -> KafkaConnectResponse: log.warning( "Rebalancing in progress while getting a connector... Retrying..." ) - sleep(1) - self.get_connector(connector_name) + await asyncio.sleep(1) + await self.get_connector(connector_name) raise KafkaConnectError(response) - def update_connector_config( + async def update_connector_config( self, connector_name: str, kafka_connect_config: KafkaConnectConfig ) -> KafkaConnectResponse: """ @@ -101,11 +106,14 @@ def update_connector_config( :return: Information about the connector after the change has been made. """ config_json = kafka_connect_config.dict(exclude_none=True) - response = requests.put( + client = httpx.AsyncClient() + response = await client.put( url=f"{self._host}/connectors/{connector_name}/config", headers=HEADERS, json=config_json, ) + await client.aclose() + data: dict = response.json() if response.status_code == requests.status_codes.codes.ok: log.info(f"Config for connector {connector_name} updated.") @@ -119,8 +127,8 @@ def update_connector_config( log.warning( "Rebalancing in progress while updating a connector... Retrying..." ) - sleep(1) - self.update_connector_config(connector_name, kafka_connect_config) + await asyncio.sleep(1) + await self.update_connector_config(connector_name, kafka_connect_config) raise KafkaConnectError(response) @classmethod @@ -133,7 +141,7 @@ def get_connector_config( connector_config["name"] = connector_name return connector_config - def validate_connector_config( + async def validate_connector_config( self, connector_name: str, kafka_connect_config: KafkaConnectConfig ) -> list[str]: """ @@ -145,12 +153,14 @@ def validate_connector_config( config_json = self.get_connector_config(connector_name, kafka_connect_config) connector_class = ConnectWrapper.get_connector_class_name(config_json) + client = httpx.AsyncClient() - response = requests.put( + response = await client.put( url=f"{self._host}/connector-plugins/{connector_class}/config/validate", headers=HEADERS, json=config_json, ) + await client.aclose() if response.status_code == requests.status_codes.codes.ok: kafka_connect_error_response = KafkaConnectConfigErrorResponse( @@ -168,14 +178,16 @@ def validate_connector_config( return errors raise KafkaConnectError(response) - def delete_connector(self, connector_name: str) -> None: + async def delete_connector(self, connector_name: str) -> None: """ Deletes a connector, halting all tasks and deleting its configuration. API Reference:https://docs.confluent.io/platform/current/connect/references/restapi.html#delete--connectors-(string-name)- """ - response = requests.delete( + client = httpx.AsyncClient() + response = await client.delete( url=f"{self._host}/connectors/{connector_name}", headers=HEADERS ) + await client.aclose() if response.status_code == requests.status_codes.codes.no_content: log.info(f"Connector {connector_name} deleted.") return @@ -186,8 +198,8 @@ def delete_connector(self, connector_name: str) -> None: log.warning( "Rebalancing in progress while deleting a connector... Retrying..." ) - sleep(1) - self.delete_connector(connector_name) + await asyncio.sleep(1) + await self.delete_connector(connector_name) raise KafkaConnectError(response) @staticmethod diff --git a/kpops/component_handlers/kafka_connect/kafka_connect_handler.py b/kpops/component_handlers/kafka_connect/kafka_connect_handler.py index b6a26260f..48b2f13df 100644 --- a/kpops/component_handlers/kafka_connect/kafka_connect_handler.py +++ b/kpops/component_handlers/kafka_connect/kafka_connect_handler.py @@ -33,7 +33,7 @@ def __init__( self._connect_wrapper = connect_wrapper self._timeout = timeout - def create_connector( + async def create_connector( self, connector_name: str, kafka_connect_config: KafkaConnectConfig, @@ -46,46 +46,66 @@ def create_connector( :param dry_run: If the connector creation should be run in dry run mode. """ if dry_run: - self.__dry_run_connector_creation(connector_name, kafka_connect_config) + await self.__dry_run_connector_creation( + connector_name, kafka_connect_config + ) else: try: - timeout( - lambda: self._connect_wrapper.get_connector(connector_name), + + async def get_connector_locally(): + return await self._connect_wrapper.get_connector(connector_name) + + await timeout( + get_connector_locally(), secs=self._timeout, ) - timeout( - lambda: self._connect_wrapper.update_connector_config( + async def update_connector_locally(): + await self._connect_wrapper.update_connector_config( connector_name, kafka_connect_config - ), + ) + + await timeout( + update_connector_locally(), secs=self._timeout, ) except ConnectorNotFoundException: - timeout( - lambda: self._connect_wrapper.create_connector( + + async def create_connecto_locally(): + await self._connect_wrapper.create_connector( connector_name, kafka_connect_config - ), + ) + + await timeout( + create_connecto_locally(), secs=self._timeout, ) - def destroy_connector(self, connector_name: str, dry_run: bool) -> None: + async def destroy_connector(self, connector_name: str, dry_run: bool) -> None: """ Deletes a connector resource from the cluster. :param connector_name: The connector name. :param dry_run: If the connector deletion should be run in dry run mode. """ if dry_run: - self.__dry_run_connector_deletion(connector_name) + await self.__dry_run_connector_deletion(connector_name) else: try: - timeout( - lambda: self._connect_wrapper.get_connector(connector_name), + + async def get_connector_locally(): + return await self._connect_wrapper.get_connector(connector_name) + + await timeout( + get_connector_locally(), secs=self._timeout, ) - timeout( - lambda: self._connect_wrapper.delete_connector(connector_name), + async def delete_connector_locally(): + await self._connect_wrapper.delete_connector(connector_name) + + await timeout( + delete_connector_locally(), secs=self._timeout, ) except ConnectorNotFoundException: @@ -93,11 +113,11 @@ def destroy_connector(self, connector_name: str, dry_run: bool) -> None: f"Connector Destruction: the connector {connector_name} does not exist. Skipping." ) - def __dry_run_connector_creation( + async def __dry_run_connector_creation( self, connector_name: str, kafka_connect_config: KafkaConnectConfig ) -> None: try: - connector_config = self._connect_wrapper.get_connector(connector_name) + connector_config = await self._connect_wrapper.get_connector(connector_name) log.info(f"Connector Creation: connector {connector_name} already exists.") if diff := render_diff( @@ -119,7 +139,7 @@ def __dry_run_connector_creation( log.debug("POST /connectors HTTP/1.1") log.debug(f"HOST: {self._connect_wrapper.host}") - errors = self._connect_wrapper.validate_connector_config( + errors = await self._connect_wrapper.validate_connector_config( connector_name, kafka_connect_config ) if len(errors) > 0: @@ -132,9 +152,9 @@ def __dry_run_connector_creation( f"Connector Creation: connector config for {connector_name} is valid!" ) - def __dry_run_connector_deletion(self, connector_name: str) -> None: + async def __dry_run_connector_deletion(self, connector_name: str) -> None: try: - self._connect_wrapper.get_connector(connector_name) + await self._connect_wrapper.get_connector(connector_name) log.info( magentaify( f"Connector Destruction: connector {connector_name} already exists. Deleting connector." diff --git a/kpops/component_handlers/kafka_connect/timeout.py b/kpops/component_handlers/kafka_connect/timeout.py index d93d608b7..f6cd5f4e3 100644 --- a/kpops/component_handlers/kafka_connect/timeout.py +++ b/kpops/component_handlers/kafka_connect/timeout.py @@ -1,32 +1,30 @@ import asyncio +import inspect import logging from asyncio import TimeoutError -from typing import Callable, TypeVar +from typing import Awaitable, Callable, TypeVar log = logging.getLogger("Timeout") T = TypeVar("T") -def timeout(func: Callable[..., T], *, secs: int = 0) -> T | None: +async def timeout(func: Callable[..., T] | Awaitable[T], *, secs: int = 0) -> T | None: """ Sets a timeout for a given lambda function :param func: The callable function :param secs: The timeout in seconds """ - - async def main_supervisor(func: Callable[..., T], secs: int) -> T: - runner = asyncio.to_thread(func) + try: + if inspect.isawaitable(func): + runner = func + else: + runner = asyncio.to_thread(func) task = asyncio.create_task(runner) if secs == 0: return await task else: return await asyncio.wait_for(task, timeout=secs) - - loop = asyncio.get_event_loop() - try: - complete = loop.run_until_complete(main_supervisor(func, secs)) - return complete except TimeoutError: log.error( f"Kafka Connect operation {func.__name__} timed out after {secs} seconds. To increase the duration, set the `timeout` option in config.yaml." diff --git a/kpops/component_handlers/topic/handler.py b/kpops/component_handlers/topic/handler.py index 9e4d7abab..2f22bef5a 100644 --- a/kpops/component_handlers/topic/handler.py +++ b/kpops/component_handlers/topic/handler.py @@ -30,7 +30,9 @@ async def create_topics(self, to_section: ToSection, dry_run: bool) -> None: for topic_name, topic_config in to_section.topics.items(): topic_spec = self.__prepare_body(topic_name, topic_config) if dry_run: - await self.__dry_run_topic_creation(topic_name, topic_spec, topic_config) + await self.__dry_run_topic_creation( + topic_name, topic_spec, topic_config + ) else: try: await self.proxy_wrapper.get_topic(topic_name=topic_name) diff --git a/kpops/components/base_components/kafka_app.py b/kpops/components/base_components/kafka_app.py index 889aeaa40..23ebec8dc 100644 --- a/kpops/components/base_components/kafka_app.py +++ b/kpops/components/base_components/kafka_app.py @@ -107,7 +107,7 @@ def clean_up_helm_chart(self) -> str: @override async def deploy(self, dry_run: bool) -> None: if self.to: - self.handlers.topic_handler.create_topics( + await self.handlers.topic_handler.create_topics( to_section=self.to, dry_run=dry_run ) diff --git a/kpops/components/base_components/kafka_connector.py b/kpops/components/base_components/kafka_connector.py index 275ba4797..21b2941ef 100644 --- a/kpops/components/base_components/kafka_connector.py +++ b/kpops/components/base_components/kafka_connector.py @@ -122,7 +122,7 @@ def kafka_connect_resetter_chart(self) -> str: @override async def deploy(self, dry_run: bool) -> None: if self.to: - self.handlers.topic_handler.create_topics( + await self.handlers.topic_handler.create_topics( to_section=self.to, dry_run=dry_run ) @@ -131,7 +131,7 @@ async def deploy(self, dry_run: bool) -> None: to_section=self.to, dry_run=dry_run ) - self.handlers.connector_handler.create_connector( + await self.handlers.connector_handler.create_connector( connector_name=self.name, kafka_connect_config=self.app, dry_run=dry_run ) diff --git a/tests/component_handlers/kafka_connect/test_connect_handler.py b/tests/component_handlers/kafka_connect/test_connect_handler.py index af4cdf228..45103133f 100644 --- a/tests/component_handlers/kafka_connect/test_connect_handler.py +++ b/tests/component_handlers/kafka_connect/test_connect_handler.py @@ -1,5 +1,5 @@ from unittest import mock -from unittest.mock import MagicMock +from unittest.mock import AsyncMock, MagicMock import pytest from pytest_mock import MockerFixture @@ -53,17 +53,18 @@ def connector_handler(connect_wrapper: MagicMock) -> KafkaConnectHandler: timeout=0, ) - def test_should_create_connector_in_dry_run( + @pytest.mark.asyncio + async def test_should_create_connector_in_dry_run( self, renderer_diff_mock: MagicMock, log_info_mock: MagicMock, ): - connector_wrapper = MagicMock() + connector_wrapper = AsyncMock() handler = self.connector_handler(connector_wrapper) renderer_diff_mock.return_value = None config = KafkaConnectConfig() - handler.create_connector(CONNECTOR_NAME, config, True) + await handler.create_connector(CONNECTOR_NAME, config, True) connector_wrapper.get_connector.assert_called_once_with(CONNECTOR_NAME) connector_wrapper.validate_connector_config.assert_called_once_with( CONNECTOR_NAME, config @@ -78,11 +79,12 @@ def test_should_create_connector_in_dry_run( ), ] - def test_should_log_correct_message_when_create_connector_and_connector_not_exists_in_dry_run( + @pytest.mark.asyncio + async def test_should_log_correct_message_when_create_connector_and_connector_not_exists_in_dry_run( self, log_info_mock: MagicMock, ): - connector_wrapper = MagicMock() + connector_wrapper = AsyncMock() handler = self.connector_handler(connector_wrapper) connector_wrapper.get_connector.side_effect = ConnectorNotFoundException() @@ -93,7 +95,7 @@ def test_should_log_correct_message_when_create_connector_and_connector_not_exis "topics": "test-topic", } config = KafkaConnectConfig(**configs) - handler.create_connector(CONNECTOR_NAME, config, True) + await handler.create_connector(CONNECTOR_NAME, config, True) connector_wrapper.get_connector.assert_called_once_with(CONNECTOR_NAME) connector_wrapper.validate_connector_config.assert_called_once_with( CONNECTOR_NAME, config @@ -108,11 +110,12 @@ def test_should_log_correct_message_when_create_connector_and_connector_not_exis ), ] - def test_should_log_correct_message_when_create_connector_and_connector_exists_in_dry_run( + @pytest.mark.asyncio + async def test_should_log_correct_message_when_create_connector_and_connector_exists_in_dry_run( self, log_info_mock: MagicMock, ): - connector_wrapper = MagicMock() + connector_wrapper = AsyncMock() handler = self.connector_handler(connector_wrapper) actual_response = { @@ -135,7 +138,7 @@ def test_should_log_correct_message_when_create_connector_and_connector_exists_i "topics": "test-topic", } config = KafkaConnectConfig(**configs) - handler.create_connector(CONNECTOR_NAME, config, True) + await handler.create_connector(CONNECTOR_NAME, config, True) connector_wrapper.get_connector.assert_called_once_with(CONNECTOR_NAME) connector_wrapper.validate_connector_config.assert_called_once_with( CONNECTOR_NAME, config @@ -153,10 +156,11 @@ def test_should_log_correct_message_when_create_connector_and_connector_exists_i ), ] - def test_should_log_invalid_config_when_create_connector_in_dry_run( + @pytest.mark.asyncio + async def test_should_log_invalid_config_when_create_connector_in_dry_run( self, renderer_diff_mock: MagicMock ): - connector_wrapper = MagicMock() + connector_wrapper = AsyncMock() errors = [ "Missing required configuration file which has no default value.", @@ -174,50 +178,53 @@ def test_should_log_invalid_config_when_create_connector_in_dry_run( ConnectorStateException, match=f"Connector Creation: validating the connector config for connector {CONNECTOR_NAME} resulted in the following errors: {formatted_errors}", ): - handler.create_connector(CONNECTOR_NAME, config, True) + await handler.create_connector(CONNECTOR_NAME, config, True) connector_wrapper.validate_connector_config.assert_called_once_with( CONNECTOR_NAME, config ) - def test_should_call_update_connector_config_when_connector_exists_not_dry_run( + @pytest.mark.asyncio + async def test_should_call_update_connector_config_when_connector_exists_not_dry_run( self, ): - connector_wrapper = MagicMock() + connector_wrapper = AsyncMock() handler = self.connector_handler(connector_wrapper) config = KafkaConnectConfig() - handler.create_connector(CONNECTOR_NAME, config, False) + await handler.create_connector(CONNECTOR_NAME, config, False) assert connector_wrapper.mock_calls == [ mock.call.get_connector(CONNECTOR_NAME), mock.call.update_connector_config(CONNECTOR_NAME, config), ] - def test_should_call_create_connector_when_connector_does_not_exists_not_dry_run( + @pytest.mark.asyncio + async def test_should_call_create_connector_when_connector_does_not_exists_not_dry_run( self, ): - connector_wrapper = MagicMock() + connector_wrapper = AsyncMock() handler = self.connector_handler(connector_wrapper) config = KafkaConnectConfig() connector_wrapper.get_connector.side_effect = ConnectorNotFoundException() - handler.create_connector(CONNECTOR_NAME, config, False) + await handler.create_connector(CONNECTOR_NAME, config, False) connector_wrapper.create_connector.assert_called_once_with( CONNECTOR_NAME, config ) - def test_should_print_correct_log_when_destroying_connector_in_dry_run( + @pytest.mark.asyncio + async def test_should_print_correct_log_when_destroying_connector_in_dry_run( self, log_info_mock: MagicMock, ): - connector_wrapper = MagicMock() + connector_wrapper = AsyncMock() handler = self.connector_handler(connector_wrapper) - handler.destroy_connector(CONNECTOR_NAME, True) + await handler.destroy_connector(CONNECTOR_NAME, True) log_info_mock.assert_called_once_with( magentaify( @@ -225,42 +232,45 @@ def test_should_print_correct_log_when_destroying_connector_in_dry_run( ) ) - def test_should_print_correct_warning_log_when_destroying_connector_and_connector_exists_in_dry_run( + @pytest.mark.asyncio + async def test_should_print_correct_warning_log_when_destroying_connector_and_connector_exists_in_dry_run( self, log_warning_mock: MagicMock, ): - connector_wrapper = MagicMock() + connector_wrapper = AsyncMock() connector_wrapper.get_connector.side_effect = ConnectorNotFoundException() handler = self.connector_handler(connector_wrapper) - handler.destroy_connector(CONNECTOR_NAME, True) + await handler.destroy_connector(CONNECTOR_NAME, True) log_warning_mock.assert_called_once_with( f"Connector Destruction: connector {CONNECTOR_NAME} does not exist and cannot be deleted. Skipping." ) - def test_should_call_delete_connector_when_destroying_existing_connector_not_dry_run( + @pytest.mark.asyncio + async def test_should_call_delete_connector_when_destroying_existing_connector_not_dry_run( self, ): - connector_wrapper = MagicMock() + connector_wrapper = AsyncMock() handler = self.connector_handler(connector_wrapper) - handler.destroy_connector(CONNECTOR_NAME, False) + await handler.destroy_connector(CONNECTOR_NAME, False) assert connector_wrapper.mock_calls == [ mock.call.get_connector(CONNECTOR_NAME), mock.call.delete_connector(CONNECTOR_NAME), ] - def test_should_print_correct_warning_log_when_destroying_connector_and_connector_exists_not_dry_run( + @pytest.mark.asyncio + async def test_should_print_correct_warning_log_when_destroying_connector_and_connector_exists_not_dry_run( self, log_warning_mock: MagicMock, ): - connector_wrapper = MagicMock() + connector_wrapper = AsyncMock() connector_wrapper.get_connector.side_effect = ConnectorNotFoundException() handler = self.connector_handler(connector_wrapper) - handler.destroy_connector(CONNECTOR_NAME, False) + await handler.destroy_connector(CONNECTOR_NAME, False) log_warning_mock.assert_called_once_with( f"Connector Destruction: the connector {CONNECTOR_NAME} does not exist. Skipping." diff --git a/tests/component_handlers/kafka_connect/test_connect_wrapper.py b/tests/component_handlers/kafka_connect/test_connect_wrapper.py index d234f6a85..5e4f5087f 100644 --- a/tests/component_handlers/kafka_connect/test_connect_wrapper.py +++ b/tests/component_handlers/kafka_connect/test_connect_wrapper.py @@ -1,11 +1,11 @@ import json import sys -import unittest from pathlib import Path from unittest.mock import MagicMock, patch import pytest -import responses +import pytest_asyncio +from pytest_httpx import HTTPXMock from kpops.cli.pipeline_config import PipelineConfig from kpops.component_handlers.kafka_connect.connect_wrapper import ConnectWrapper @@ -25,9 +25,9 @@ DEFAULTS_PATH = Path(__file__).parent / "resources" -class TestConnectorApiWrapper(unittest.TestCase): - @pytest.fixture(autouse=True) - def setup(self): +class TestConnectorApiWrapper: + @pytest_asyncio.fixture(autouse=True) + async def setup(self): config = PipelineConfig( defaults_path=DEFAULTS_PATH, environment="development", @@ -48,8 +48,9 @@ def test_should_through_exception_when_host_is_not_set(self): == "The Kafka Connect host is not set. Please set the host in the config." ) - @patch("requests.post") - def test_should_create_post_requests_for_given_connector_configuration( + @pytest.mark.asyncio + @patch("httpx.AsyncClient.post") + async def test_should_create_post_requests_for_given_connector_configuration( self, mock_post: MagicMock ): configs = { @@ -63,7 +64,7 @@ def test_should_create_post_requests_for_given_connector_configuration( } with pytest.raises(KafkaConnectError): - self.connect_wrapper.create_connector( + await self.connect_wrapper.create_connector( "test-connector", kafka_connect_config=KafkaConnectConfig(**configs) ) @@ -76,8 +77,10 @@ def test_should_create_post_requests_for_given_connector_configuration( }, ) - @responses.activate - def test_should_return_correct_response_when_connector_created(self): + @pytest.mark.asyncio + async def test_should_return_correct_response_when_connector_created( + self, httpx_mock + ): actual_response = { "name": "hdfs-sink-connector", "config": { @@ -96,34 +99,37 @@ def test_should_return_correct_response_when_connector_created(self): {"connector": "hdfs-sink-connector", "task": 3}, ], } - responses.add( - responses.POST, - f"{HOST}/connectors", + httpx_mock.add_response( + method="POST", + url=f"{HOST}/connectors", headers=HEADERS, json=actual_response, - status=201, + status_code=201, ) - expected_response = self.connect_wrapper.create_connector( + expected_response = await self.connect_wrapper.create_connector( "test-connector", kafka_connect_config=KafkaConnectConfig() ) - self.assertEqual(KafkaConnectResponse(**actual_response), expected_response) + assert KafkaConnectResponse(**actual_response) == expected_response - @responses.activate + @pytest.mark.asyncio @patch("kpops.component_handlers.kafka_connect.connect_wrapper.log.warning") - def test_should_raise_connector_exists_exception_when_connector_exists( - self, log_warning: MagicMock + async def test_should_raise_connector_exists_exception_when_connector_exists( + self, log_warning: MagicMock, httpx_mock: HTTPXMock ): - responses.add( - responses.POST, - f"{HOST}/connectors", + httpx_mock.add_response( + method="POST", + url=f"{HOST}/connectors", json={}, - status=409, + status_code=409, ) - timeout( - lambda: self.connect_wrapper.create_connector( + async def create_connector_locally(): + await self.connect_wrapper.create_connector( "test-name", KafkaConnectConfig() - ), + ) + + await timeout( + create_connector_locally(), secs=1, ) @@ -131,11 +137,14 @@ def test_should_raise_connector_exists_exception_when_connector_exists( "Rebalancing in progress while creating a connector... Retrying..." ) - @patch("requests.get") - def test_should_create_correct_get_connector_request(self, mock_get: MagicMock): + @pytest.mark.asyncio + @patch("httpx.AsyncClient.get") + async def test_should_create_correct_get_connector_request( + self, mock_get: MagicMock + ): connector_name = "test-connector" with pytest.raises(KafkaConnectError): - self.connect_wrapper.get_connector(connector_name) + await self.connect_wrapper.get_connector(connector_name) mock_get.assert_called_with( url=f"{HOST}/connectors/{connector_name}", @@ -143,10 +152,10 @@ def test_should_create_correct_get_connector_request(self, mock_get: MagicMock): ) @pytest.mark.flaky(reruns=5, condition=sys.platform.startswith("win32")) - @responses.activate + @pytest.mark.asyncio @patch("kpops.component_handlers.kafka_connect.connect_wrapper.log.info") - def test_should_return_correct_response_when_getting_connector( - self, log_info: MagicMock + async def test_should_return_correct_response_when_getting_connector( + self, log_info: MagicMock, httpx_mock: HTTPXMock ): connector_name = "test-connector" @@ -168,39 +177,40 @@ def test_should_return_correct_response_when_getting_connector( {"connector": "hdfs-sink-connector", "task": 3}, ], } - responses.add( - responses.GET, - f"{HOST}/connectors/{connector_name}", + httpx_mock.add_response( + method="GET", + url=f"{HOST}/connectors/{connector_name}", headers=HEADERS, json=actual_response, - status=200, + status_code=200, ) - expected_response = self.connect_wrapper.get_connector(connector_name) - self.assertEqual(KafkaConnectResponse(**actual_response), expected_response) + expected_response = await self.connect_wrapper.get_connector(connector_name) + assert KafkaConnectResponse(**actual_response) == expected_response log_info.assert_called_once_with(f"Connector {connector_name} exists.") - @responses.activate + @pytest.mark.asyncio @patch("kpops.component_handlers.kafka_connect.connect_wrapper.log.info") - def test_should_raise_connector_not_found_when_getting_connector( - self, log_info: MagicMock + async def test_should_raise_connector_not_found_when_getting_connector( + self, log_info: MagicMock, httpx_mock: HTTPXMock ): connector_name = "test-connector" - responses.add( - responses.GET, - f"{HOST}/connectors/{connector_name}", + httpx_mock.add_response( + method="GET", + url=f"{HOST}/connectors/{connector_name}", headers=HEADERS, json={}, - status=404, + status_code=404, ) with pytest.raises(ConnectorNotFoundException): - self.connect_wrapper.get_connector(connector_name) + await self.connect_wrapper.get_connector(connector_name) log_info.assert_called_once_with( f"The named connector {connector_name} does not exists." ) - def test_should_raise_connector_name_do_not_match(self): + @pytest.mark.asyncio + async def test_should_raise_connector_name_do_not_match(self): configs = { "connector.class": "org.apache.kafka.connect.file.FileStreamSinkConnector", "tasks.max": "1", @@ -208,7 +218,7 @@ def test_should_raise_connector_name_do_not_match(self): "name": "OtherWrongName", } with pytest.raises(ValueError) as value_error: - self.connect_wrapper.validate_connector_config( + await self.connect_wrapper.validate_connector_config( "FileStreamSinkConnector", kafka_connect_config=KafkaConnectConfig(**configs), ) @@ -218,23 +228,26 @@ def test_should_raise_connector_name_do_not_match(self): == "Connector name should be the same as component name" ) - @responses.activate + @pytest.mark.asyncio @patch("kpops.component_handlers.kafka_connect.connect_wrapper.log.warning") - def test_should_raise_rebalance_in_progress_when_getting_connector( - self, log_warning: MagicMock + async def test_should_raise_rebalance_in_progress_when_getting_connector( + self, log_warning: MagicMock, httpx_mock: HTTPXMock ): connector_name = "test-connector" - responses.add( - responses.GET, - f"{HOST}/connectors/{connector_name}", + httpx_mock.add_response( + method="GET", + url=f"{HOST}/connectors/{connector_name}", headers=HEADERS, json={}, - status=409, + status_code=409, ) - timeout( - lambda: self.connect_wrapper.get_connector(connector_name), + async def get_connector_locally(): + return await self.connect_wrapper.get_connector(connector_name) + + await timeout( + get_connector_locally(), secs=1, ) @@ -242,8 +255,11 @@ def test_should_raise_rebalance_in_progress_when_getting_connector( "Rebalancing in progress while getting a connector... Retrying..." ) - @patch("requests.put") - def test_should_create_correct_update_connector_request(self, mock_put: MagicMock): + @pytest.mark.asyncio + @patch("httpx.AsyncClient.put") + async def test_should_create_correct_update_connector_request( + self, mock_put: MagicMock + ): connector_name = "test-connector" configs = { "batch.size": "50", @@ -255,7 +271,7 @@ def test_should_create_correct_update_connector_request(self, mock_put: MagicMoc "connector.class": "io.confluent.connect.elasticsearch.ElasticsearchSinkConnector", } with pytest.raises(KafkaConnectError): - self.connect_wrapper.update_connector_config( + await self.connect_wrapper.update_connector_config( connector_name, KafkaConnectConfig(**configs), ) @@ -266,10 +282,10 @@ def test_should_create_correct_update_connector_request(self, mock_put: MagicMoc json=KafkaConnectConfig(**configs).dict(), ) - @responses.activate + @pytest.mark.asyncio @patch("kpops.component_handlers.kafka_connect.connect_wrapper.log.info") - def test_should_return_correct_response_when_update_connector( - self, log_info: MagicMock + async def test_should_return_correct_response_when_update_connector( + self, log_info: MagicMock, httpx_mock: HTTPXMock ): connector_name = "test-connector" @@ -291,25 +307,25 @@ def test_should_return_correct_response_when_update_connector( {"connector": "hdfs-sink-connector", "task": 3}, ], } - responses.add( - responses.PUT, - f"{HOST}/connectors/{connector_name}/config", + httpx_mock.add_response( + method="PUT", + url=f"{HOST}/connectors/{connector_name}/config", headers=HEADERS, json=actual_response, - status=200, + status_code=200, ) - expected_response = self.connect_wrapper.update_connector_config( + expected_response = await self.connect_wrapper.update_connector_config( connector_name, KafkaConnectConfig() ) - self.assertEqual(KafkaConnectResponse(**actual_response), expected_response) + assert KafkaConnectResponse(**actual_response) == expected_response log_info.assert_called_once_with( f"Config for connector {connector_name} updated." ) - @responses.activate + @pytest.mark.asyncio @patch("kpops.component_handlers.kafka_connect.connect_wrapper.log.info") - def test_should_return_correct_response_when_update_connector_created( - self, log_info: MagicMock + async def test_should_return_correct_response_when_update_connector_created( + self, log_info: MagicMock, httpx_mock: HTTPXMock ): connector_name = "test-connector" @@ -331,38 +347,41 @@ def test_should_return_correct_response_when_update_connector_created( {"connector": "hdfs-sink-connector", "task": 3}, ], } - responses.add( - responses.PUT, - f"{HOST}/connectors/{connector_name}/config", + httpx_mock.add_response( + method="PUT", + url=f"{HOST}/connectors/{connector_name}/config", headers=HEADERS, json=actual_response, - status=201, + status_code=201, ) - expected_response = self.connect_wrapper.update_connector_config( + expected_response = await self.connect_wrapper.update_connector_config( connector_name, KafkaConnectConfig() ) - self.assertEqual(KafkaConnectResponse(**actual_response), expected_response) + assert KafkaConnectResponse(**actual_response) == expected_response log_info.assert_called_once_with(f"Connector {connector_name} created.") - @responses.activate + @pytest.mark.asyncio @patch("kpops.component_handlers.kafka_connect.connect_wrapper.log.warning") - def test_should_raise_connector_exists_exception_when_update_connector( - self, log_warning: MagicMock + async def test_should_raise_connector_exists_exception_when_update_connector( + self, log_warning: MagicMock, httpx_mock: HTTPXMock ): connector_name = "test-connector" - responses.add( - responses.PUT, - f"{HOST}/connectors/{connector_name}/config", + httpx_mock.add_response( + method="PUT", + url=f"{HOST}/connectors/{connector_name}/config", headers=HEADERS, json={}, - status=409, + status_code=409, ) - timeout( - lambda: self.connect_wrapper.update_connector_config( + async def update_connector_locally(): + await self.connect_wrapper.update_connector_config( connector_name, KafkaConnectConfig() - ), + ) + + await timeout( + update_connector_locally(), secs=1, ) @@ -370,23 +389,24 @@ def test_should_raise_connector_exists_exception_when_update_connector( "Rebalancing in progress while updating a connector... Retrying..." ) - @patch("requests.delete") - def test_should_create_correct_delete_connector_request( + @pytest.mark.asyncio + @patch("httpx.AsyncClient.delete") + async def test_should_create_correct_delete_connector_request( self, mock_delete: MagicMock ): connector_name = "test-connector" with pytest.raises(KafkaConnectError): - self.connect_wrapper.delete_connector(connector_name) + await self.connect_wrapper.delete_connector(connector_name) mock_delete.assert_called_with( url=f"{HOST}/connectors/{connector_name}", headers=HEADERS, ) - @responses.activate + @pytest.mark.asyncio @patch("kpops.component_handlers.kafka_connect.connect_wrapper.log.info") - def test_should_return_correct_response_when_deleting_connector( - self, log_info: MagicMock + async def test_should_return_correct_response_when_deleting_connector( + self, log_info: MagicMock, httpx_mock: HTTPXMock ): connector_name = "test-connector" @@ -408,55 +428,58 @@ def test_should_return_correct_response_when_deleting_connector( {"connector": "hdfs-sink-connector", "task": 3}, ], } - responses.add( - responses.DELETE, - f"{HOST}/connectors/{connector_name}", + httpx_mock.add_response( + method="DELETE", + url=f"{HOST}/connectors/{connector_name}", headers=HEADERS, json=actual_response, - status=204, + status_code=204, ) - self.connect_wrapper.delete_connector(connector_name) + await self.connect_wrapper.delete_connector(connector_name) log_info.assert_called_once_with(f"Connector {connector_name} deleted.") - @responses.activate + @pytest.mark.asyncio @patch("kpops.component_handlers.kafka_connect.connect_wrapper.log.info") - def test_should_raise_connector_not_found_when_deleting_connector( - self, log_info: MagicMock + async def test_should_raise_connector_not_found_when_deleting_connector( + self, log_info: MagicMock, httpx_mock: HTTPXMock ): connector_name = "test-connector" - responses.add( - responses.DELETE, - f"{HOST}/connectors/{connector_name}", + httpx_mock.add_response( + method="DELETE", + url=f"{HOST}/connectors/{connector_name}", headers=HEADERS, json={}, - status=404, + status_code=404, ) with pytest.raises(ConnectorNotFoundException): - self.connect_wrapper.delete_connector(connector_name) + await self.connect_wrapper.delete_connector(connector_name) log_info.assert_called_once_with( f"The named connector {connector_name} does not exists." ) - @responses.activate + @pytest.mark.asyncio @patch("kpops.component_handlers.kafka_connect.connect_wrapper.log.warning") - def test_should_raise_rebalance_in_progress_when_deleting_connector( - self, log_warning: MagicMock + async def test_should_raise_rebalance_in_progress_when_deleting_connector( + self, log_warning: MagicMock, httpx_mock: HTTPXMock ): connector_name = "test-connector" - responses.add( - responses.DELETE, - f"{HOST}/connectors/{connector_name}", + httpx_mock.add_response( + method="DELETE", + url=f"{HOST}/connectors/{connector_name}", headers=HEADERS, json={}, - status=409, + status_code=409, ) - timeout( - lambda: self.connect_wrapper.delete_connector(connector_name), + async def delete_connector_locally(): + await self.connect_wrapper.delete_connector(connector_name) + + await timeout( + delete_connector_locally(), secs=1, ) @@ -464,8 +487,9 @@ def test_should_raise_rebalance_in_progress_when_deleting_connector( "Rebalancing in progress while deleting a connector... Retrying..." ) - @patch("requests.put") - def test_should_create_correct_validate_connector_config_request( + @pytest.mark.asyncio + @patch("httpx.AsyncClient.put") + async def test_should_create_correct_validate_connector_config_request( self, mock_put: MagicMock ): configs = { @@ -475,7 +499,7 @@ def test_should_create_correct_validate_connector_config_request( "name": "FileStreamSinkConnector", } with pytest.raises(KafkaConnectError): - self.connect_wrapper.validate_connector_config( + await self.connect_wrapper.validate_connector_config( "FileStreamSinkConnector", kafka_connect_config=KafkaConnectConfig(**configs), ) @@ -486,8 +510,9 @@ def test_should_create_correct_validate_connector_config_request( json=KafkaConnectConfig(**configs).dict(), ) - @patch("requests.put") - def test_should_create_correct_validate_connector_config_and_name_gets_added( + @pytest.mark.asyncio + @patch("httpx.AsyncClient.put") + async def test_should_create_correct_validate_connector_config_and_name_gets_added( self, mock_put: MagicMock ): connector_name = "FileStreamSinkConnector" @@ -497,7 +522,7 @@ def test_should_create_correct_validate_connector_config_and_name_gets_added( "topics": "test-topic", } with pytest.raises(KafkaConnectError): - self.connect_wrapper.validate_connector_config( + await self.connect_wrapper.validate_connector_config( connector_name, kafka_connect_config=KafkaConnectConfig(**configs), ) @@ -508,18 +533,19 @@ def test_should_create_correct_validate_connector_config_and_name_gets_added( json=KafkaConnectConfig(**{"name": connector_name, **configs}).dict(), ) - @responses.activate - def test_should_parse_validate_connector_config(self): + @pytest.mark.asyncio + async def test_should_parse_validate_connector_config(self, httpx_mock: HTTPXMock): with open( DEFAULTS_PATH / "connect_validation_response.json", ) as f: actual_response = json.load(f) - responses.add( - responses.PUT, - f"{HOST}/connector-plugins/FileStreamSinkConnector/config/validate", + + httpx_mock.add_response( + method="PUT", + url=f"{HOST}/connector-plugins/FileStreamSinkConnector/config/validate", headers=HEADERS, json=actual_response, - status=200, + status_code=200, ) configs = { @@ -527,7 +553,7 @@ def test_should_parse_validate_connector_config(self): "tasks.max": "1", "topics": "test-topic", } - errors = self.connect_wrapper.validate_connector_config( + errors = await self.connect_wrapper.validate_connector_config( "FileStreamSinkConnector", kafka_connect_config=KafkaConnectConfig(**configs), ) diff --git a/tests/component_handlers/topic/test_topic_handler.py b/tests/component_handlers/topic/test_topic_handler.py index 63515b0aa..57fa79f99 100644 --- a/tests/component_handlers/topic/test_topic_handler.py +++ b/tests/component_handlers/topic/test_topic_handler.py @@ -2,7 +2,7 @@ import logging from pathlib import Path from unittest import mock -from unittest.mock import MagicMock, AsyncMock +from unittest.mock import AsyncMock, MagicMock import pytest import pytest_asyncio From d00939830ce8c92b0d110155d5a55e5a1cf67f0b Mon Sep 17 00:00:00 2001 From: Alejandro Jaramillo Date: Wed, 26 Jul 2023 10:41:00 +0200 Subject: [PATCH 06/23] Functional tests with partial async --- tests/components/test_producer_app.py | 8 ++++---- tests/components/test_streams_app.py | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/components/test_producer_app.py b/tests/components/test_producer_app.py index a1596350b..523e4a75b 100644 --- a/tests/components/test_producer_app.py +++ b/tests/components/test_producer_app.py @@ -1,6 +1,6 @@ import logging from pathlib import Path -from unittest.mock import ANY, MagicMock +from unittest.mock import ANY, MagicMock, AsyncMock import pytest from pytest_mock import MockerFixture @@ -27,9 +27,9 @@ class TestProducerApp: @pytest.fixture def handlers(self) -> ComponentHandlers: return ComponentHandlers( - schema_handler=MagicMock(), - connector_handler=MagicMock(), - topic_handler=MagicMock(), + schema_handler=AsyncMock(), + connector_handler=AsyncMock(), + topic_handler=AsyncMock(), ) @pytest.fixture diff --git a/tests/components/test_streams_app.py b/tests/components/test_streams_app.py index 54bff933b..e8b311d01 100644 --- a/tests/components/test_streams_app.py +++ b/tests/components/test_streams_app.py @@ -1,5 +1,5 @@ from pathlib import Path -from unittest.mock import MagicMock +from unittest.mock import MagicMock, AsyncMock import pytest from pytest_mock import MockerFixture @@ -29,9 +29,9 @@ class TestStreamsApp: @pytest.fixture def handlers(self) -> ComponentHandlers: return ComponentHandlers( - schema_handler=MagicMock(), - connector_handler=MagicMock(), - topic_handler=MagicMock(), + schema_handler=AsyncMock(), + connector_handler=AsyncMock(), + topic_handler=AsyncMock(), ) @pytest.fixture From 1ac3df733d8481d47f8873fdefbc77ab47600e01 Mon Sep 17 00:00:00 2001 From: Alejandro Jaramillo Date: Mon, 31 Jul 2023 13:51:21 +0200 Subject: [PATCH 07/23] Fixed tests --- kpops/cli/main.py | 34 +++++++++++------ .../kafka_connect/timeout.py | 11 ++---- .../base_components/kafka_connector.py | 20 +++++----- .../base_components/kubernetes_app.py | 2 +- .../base_components/pipeline_component.py | 6 +-- .../producer/producer_app.py | 2 +- .../streams_bootstrap/streams/streams_app.py | 4 +- .../topic/test_proxy_wrapper.py | 2 +- tests/components/test_kafka_sink_connector.py | 37 +++++++++++-------- .../components/test_kafka_source_connector.py | 37 +++++++++++-------- tests/components/test_kubernetes_app.py | 13 ++++--- tests/components/test_producer_app.py | 17 +++++---- tests/components/test_streams_app.py | 17 +++++---- 13 files changed, 115 insertions(+), 87 deletions(-) diff --git a/kpops/cli/main.py b/kpops/cli/main.py index 2696f828d..d3ad71b30 100644 --- a/kpops/cli/main.py +++ b/kpops/cli/main.py @@ -302,9 +302,13 @@ def destroy( pipeline_base_dir, pipeline_path, components_module, pipeline_config ) pipeline_steps = reverse_pipeline_steps(pipeline, steps) - for component in pipeline_steps: - log_action("Destroy", component) - component.destroy(dry_run) + + async def async_destroy(): + for component in pipeline_steps: + log_action("Destroy", component) + await component.destroy(dry_run) + + asyncio.run(async_destroy()) @app.command(help="Reset pipeline steps") @@ -323,10 +327,14 @@ def reset( pipeline_base_dir, pipeline_path, components_module, pipeline_config ) pipeline_steps = reverse_pipeline_steps(pipeline, steps) - for component in pipeline_steps: - log_action("Reset", component) - component.destroy(dry_run) - component.reset(dry_run) + + async def async_reset(): + for component in pipeline_steps: + log_action("Reset", component) + await component.destroy(dry_run) + await component.reset(dry_run) + + asyncio.run(async_reset()) @app.command(help="Clean pipeline steps") @@ -345,10 +353,14 @@ def clean( pipeline_base_dir, pipeline_path, components_module, pipeline_config ) pipeline_steps = reverse_pipeline_steps(pipeline, steps) - for component in pipeline_steps: - log_action("Clean", component) - component.destroy(dry_run) - component.clean(dry_run) + + async def async_clean(): + for component in pipeline_steps: + log_action("Clean", component) + await component.destroy(dry_run) + await component.clean(dry_run) + + asyncio.run(async_clean()) def version_callback(show_version: bool) -> None: diff --git a/kpops/component_handlers/kafka_connect/timeout.py b/kpops/component_handlers/kafka_connect/timeout.py index f6cd5f4e3..6822feaa8 100644 --- a/kpops/component_handlers/kafka_connect/timeout.py +++ b/kpops/component_handlers/kafka_connect/timeout.py @@ -1,26 +1,21 @@ import asyncio -import inspect import logging from asyncio import TimeoutError -from typing import Awaitable, Callable, TypeVar +from typing import Any, Coroutine, TypeVar log = logging.getLogger("Timeout") T = TypeVar("T") -async def timeout(func: Callable[..., T] | Awaitable[T], *, secs: int = 0) -> T | None: +async def timeout(func: Coroutine[Any, Any, T], *, secs: int = 0) -> T | None: """ Sets a timeout for a given lambda function :param func: The callable function :param secs: The timeout in seconds """ try: - if inspect.isawaitable(func): - runner = func - else: - runner = asyncio.to_thread(func) - task = asyncio.create_task(runner) + task = asyncio.create_task(func) if secs == 0: return await task else: diff --git a/kpops/components/base_components/kafka_connector.py b/kpops/components/base_components/kafka_connector.py index 21b2941ef..31d1a2afa 100644 --- a/kpops/components/base_components/kafka_connector.py +++ b/kpops/components/base_components/kafka_connector.py @@ -136,19 +136,19 @@ async def deploy(self, dry_run: bool) -> None: ) @override - def destroy(self, dry_run: bool) -> None: - self.handlers.connector_handler.destroy_connector( + async def destroy(self, dry_run: bool) -> None: + await self.handlers.connector_handler.destroy_connector( connector_name=self.name, dry_run=dry_run ) @override - def clean(self, dry_run: bool) -> None: + async def clean(self, dry_run: bool) -> None: if self.to: if self.handlers.schema_handler: self.handlers.schema_handler.delete_schemas( to_section=self.to, dry_run=dry_run ) - self.handlers.topic_handler.delete_topics(self.to, dry_run=dry_run) + await self.handlers.topic_handler.delete_topics(self.to, dry_run=dry_run) def _run_connect_resetter( self, @@ -348,12 +348,12 @@ def template( print(stdout) @override - def reset(self, dry_run: bool) -> None: + async def reset(self, dry_run: bool) -> None: self.__run_kafka_connect_resetter(dry_run) @override - def clean(self, dry_run: bool) -> None: - super().clean(dry_run) + async def clean(self, dry_run: bool) -> None: + await super().clean(dry_run) self.__run_kafka_connect_resetter(dry_run) def __run_kafka_connect_resetter(self, dry_run: bool) -> None: @@ -425,12 +425,12 @@ def set_error_topic(self, topic_name: str) -> None: setattr(self.app, "errors.deadletterqueue.topic.name", topic_name) @override - def reset(self, dry_run: bool) -> None: + async def reset(self, dry_run: bool) -> None: self.__run_kafka_connect_resetter(dry_run, delete_consumer_group=False) @override - def clean(self, dry_run: bool) -> None: - super().clean(dry_run) + async def clean(self, dry_run: bool) -> None: + await super().clean(dry_run) self.__run_kafka_connect_resetter(dry_run, delete_consumer_group=True) def __run_kafka_connect_resetter( diff --git a/kpops/components/base_components/kubernetes_app.py b/kpops/components/base_components/kubernetes_app.py index c890215db..f30b0b24c 100644 --- a/kpops/components/base_components/kubernetes_app.py +++ b/kpops/components/base_components/kubernetes_app.py @@ -142,7 +142,7 @@ async def deploy(self, dry_run: bool) -> None: self.dry_run_handler.print_helm_diff(stdout, self.helm_release_name, log) @override - def destroy(self, dry_run: bool) -> None: + async def destroy(self, dry_run: bool) -> None: stdout = self.helm.uninstall( self.namespace, self.helm_release_name, diff --git a/kpops/components/base_components/pipeline_component.py b/kpops/components/base_components/pipeline_component.py index 6e3e94cea..33eeb80ba 100644 --- a/kpops/components/base_components/pipeline_component.py +++ b/kpops/components/base_components/pipeline_component.py @@ -240,21 +240,21 @@ async def deploy(self, dry_run: bool) -> None: :type dry_run: bool """ - def destroy(self, dry_run: bool) -> None: + async def destroy(self, dry_run: bool) -> None: """Uninstall the component (self) from the k8s cluster :param dry_run: Whether to do a dry run of the command :type dry_run: bool """ - def reset(self, dry_run: bool) -> None: + async def reset(self, dry_run: bool) -> None: """Reset component (self) state :param dry_run: Whether to do a dry run of the command :type dry_run: bool """ - def clean(self, dry_run: bool) -> None: + async def clean(self, dry_run: bool) -> None: """Remove component (self) and any trace of it :param dry_run: Whether to do a dry run of the command diff --git a/kpops/components/streams_bootstrap/producer/producer_app.py b/kpops/components/streams_bootstrap/producer/producer_app.py index 9b605c0f2..36ae116eb 100644 --- a/kpops/components/streams_bootstrap/producer/producer_app.py +++ b/kpops/components/streams_bootstrap/producer/producer_app.py @@ -81,7 +81,7 @@ def clean_up_helm_chart(self) -> str: ) @override - def clean(self, dry_run: bool) -> None: + async def clean(self, dry_run: bool) -> None: self._run_clean_up_job( values=self.to_helm_values(), dry_run=dry_run, diff --git a/kpops/components/streams_bootstrap/streams/streams_app.py b/kpops/components/streams_bootstrap/streams/streams_app.py index f6b34ebdf..b881b107b 100644 --- a/kpops/components/streams_bootstrap/streams/streams_app.py +++ b/kpops/components/streams_bootstrap/streams/streams_app.py @@ -80,11 +80,11 @@ def clean_up_helm_chart(self) -> str: return f"{self.repo_config.repository_name}/{AppType.CLEANUP_STREAMS_APP.value}" @override - def reset(self, dry_run: bool) -> None: + async def reset(self, dry_run: bool) -> None: self.__run_streams_clean_up_job(dry_run, delete_output=False) @override - def clean(self, dry_run: bool) -> None: + 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: diff --git a/tests/component_handlers/topic/test_proxy_wrapper.py b/tests/component_handlers/topic/test_proxy_wrapper.py index 3224f0f03..34edbd74f 100644 --- a/tests/component_handlers/topic/test_proxy_wrapper.py +++ b/tests/component_handlers/topic/test_proxy_wrapper.py @@ -49,7 +49,7 @@ async def setup(self, httpx_mock: HTTPXMock): status_code=200, ) assert self.proxy_wrapper.host == HOST - assert await self.proxy_wrapper.cluster_id == "cluster-1" + assert await self.proxy_wrapper.cluster_id == "cluster-1" # type: ignore @pytest.mark.asyncio async def test_should_raise_exception_when_host_is_not_set(self): diff --git a/tests/components/test_kafka_sink_connector.py b/tests/components/test_kafka_sink_connector.py index 7e353773f..74356f51e 100644 --- a/tests/components/test_kafka_sink_connector.py +++ b/tests/components/test_kafka_sink_connector.py @@ -1,5 +1,5 @@ from pathlib import Path -from unittest.mock import MagicMock, call, AsyncMock +from unittest.mock import AsyncMock, MagicMock, call import pytest from pytest_mock import MockerFixture @@ -180,7 +180,8 @@ async def test_deploy_order( ), ] - def test_destroy( + @pytest.mark.asyncio + async def test_destroy( self, connector: KafkaSinkConnector, mocker: MockerFixture, @@ -189,24 +190,26 @@ def test_destroy( connector.handlers.connector_handler, "destroy_connector" ) - connector.destroy(dry_run=True) + await connector.destroy(dry_run=True) mock_destroy_connector.assert_called_once_with( connector_name=CONNECTOR_NAME, dry_run=True, ) - def test_reset_when_dry_run_is_true( + @pytest.mark.asyncio + async def test_reset_when_dry_run_is_true( self, connector: KafkaSinkConnector, dry_run_handler: MagicMock, ): dry_run = True - connector.reset(dry_run=dry_run) + await connector.reset(dry_run=dry_run) dry_run_handler.print_helm_diff.assert_called_once() - def test_reset_when_dry_run_is_false( + @pytest.mark.asyncio + async def test_reset_when_dry_run_is_false( self, connector: KafkaSinkConnector, helm_mock: MagicMock, @@ -224,7 +227,7 @@ def test_reset_when_dry_run_is_false( mock.attach_mock(helm_mock, "helm") dry_run = False - connector.reset(dry_run=dry_run) + await connector.reset(dry_run=dry_run) assert mock.mock_calls == [ mocker.call.helm.add_repo( @@ -267,16 +270,18 @@ def test_reset_when_dry_run_is_false( dry_run_handler.print_helm_diff.assert_not_called() mock_delete_topics.assert_not_called() - def test_clean_when_dry_run_is_true( + @pytest.mark.asyncio + async def test_clean_when_dry_run_is_true( self, connector: KafkaSinkConnector, dry_run_handler: MagicMock, ): dry_run = True - connector.clean(dry_run=dry_run) + await connector.clean(dry_run=dry_run) dry_run_handler.print_helm_diff.assert_called_once() - def test_clean_when_dry_run_is_false( + @pytest.mark.asyncio + async def test_clean_when_dry_run_is_false( self, connector: KafkaSinkConnector, config: PipelineConfig, @@ -314,7 +319,7 @@ def test_clean_when_dry_run_is_false( mock.attach_mock(helm_mock, "helm") dry_run = False - connector.clean(dry_run=dry_run) + await connector.clean(dry_run=dry_run) assert log_info_mock.mock_calls == [ call.log_info( @@ -370,7 +375,8 @@ def test_clean_when_dry_run_is_false( ] dry_run_handler.print_helm_diff.assert_not_called() - def test_clean_without_to_when_dry_run_is_true( + @pytest.mark.asyncio + async def test_clean_without_to_when_dry_run_is_true( self, config: PipelineConfig, handlers: ComponentHandlers, @@ -385,10 +391,11 @@ def test_clean_without_to_when_dry_run_is_true( ) dry_run = True - connector.clean(dry_run) + await connector.clean(dry_run) dry_run_handler.print_helm_diff.assert_called_once() - def test_clean_without_to_when_dry_run_is_false( + @pytest.mark.asyncio + async def test_clean_without_to_when_dry_run_is_false( self, config: PipelineConfig, handlers: ComponentHandlers, @@ -416,7 +423,7 @@ def test_clean_without_to_when_dry_run_is_false( mock.attach_mock(helm_mock, "helm") dry_run = False - connector.clean(dry_run) + await connector.clean(dry_run) assert mock.mock_calls == [ mocker.call.helm.add_repo( diff --git a/tests/components/test_kafka_source_connector.py b/tests/components/test_kafka_source_connector.py index d50280e4e..7961d6eae 100644 --- a/tests/components/test_kafka_source_connector.py +++ b/tests/components/test_kafka_source_connector.py @@ -1,5 +1,5 @@ from pathlib import Path -from unittest.mock import MagicMock, AsyncMock +from unittest.mock import AsyncMock, MagicMock import pytest from pytest_mock import MockerFixture @@ -131,7 +131,8 @@ async def test_deploy_order( ), ] - def test_destroy( + @pytest.mark.asyncio + async def test_destroy( self, connector: KafkaSourceConnector, mocker: MockerFixture, @@ -143,25 +144,27 @@ def test_destroy( connector.handlers.connector_handler, "destroy_connector" ) - connector.destroy(dry_run=True) + await connector.destroy(dry_run=True) mock_destroy_connector.assert_called_once_with( connector_name=CONNECTOR_NAME, dry_run=True, ) - def test_reset_when_dry_run_is_true( + @pytest.mark.asyncio + async def test_reset_when_dry_run_is_true( self, connector: KafkaSourceConnector, dry_run_handler: MagicMock, ): assert connector.handlers.connector_handler - connector.reset(dry_run=True) + await connector.reset(dry_run=True) dry_run_handler.print_helm_diff.assert_called_once() - def test_reset_when_dry_run_is_false( + @pytest.mark.asyncio + async def test_reset_when_dry_run_is_false( self, connector: KafkaSourceConnector, dry_run_handler: MagicMock, @@ -180,7 +183,7 @@ def test_reset_when_dry_run_is_false( mock.attach_mock(mock_clean_connector, "mock_clean_connector") mock.attach_mock(helm_mock, "helm") - connector.reset(dry_run=False) + await connector.reset(dry_run=False) assert mock.mock_calls == [ mocker.call.helm.add_repo( @@ -222,18 +225,20 @@ def test_reset_when_dry_run_is_false( mock_delete_topics.assert_not_called() dry_run_handler.print_helm_diff.assert_not_called() - def test_clean_when_dry_run_is_true( + @pytest.mark.asyncio + async def test_clean_when_dry_run_is_true( self, connector: KafkaSourceConnector, dry_run_handler: MagicMock, ): assert connector.handlers.connector_handler - connector.clean(dry_run=True) + await connector.clean(dry_run=True) dry_run_handler.print_helm_diff.assert_called_once() - def test_clean_when_dry_run_is_false( + @pytest.mark.asyncio + async def test_clean_when_dry_run_is_false( self, connector: KafkaSourceConnector, helm_mock: MagicMock, @@ -254,7 +259,7 @@ def test_clean_when_dry_run_is_false( mock.attach_mock(mock_clean_connector, "mock_clean_connector") mock.attach_mock(helm_mock, "helm") - connector.clean(dry_run=False) + await connector.clean(dry_run=False) assert mock.mock_calls == [ mocker.call.mock_delete_topics(connector.to, dry_run=False), @@ -297,7 +302,8 @@ def test_clean_when_dry_run_is_false( dry_run_handler.print_helm_diff.assert_not_called() - def test_clean_without_to_when_dry_run_is_false( + @pytest.mark.asyncio + async def test_clean_without_to_when_dry_run_is_false( self, config: PipelineConfig, handlers: ComponentHandlers, @@ -329,7 +335,7 @@ def test_clean_without_to_when_dry_run_is_false( mock.attach_mock(mock_clean_connector, "mock_clean_connector") mock.attach_mock(helm_mock, "helm") - connector.clean(dry_run=False) + await connector.clean(dry_run=False) assert mock.mock_calls == [ mocker.call.helm.add_repo( @@ -372,7 +378,8 @@ def test_clean_without_to_when_dry_run_is_false( mock_delete_topics.assert_not_called() dry_run_handler.print_helm_diff.assert_not_called() - def test_clean_without_to_when_dry_run_is_true( + @pytest.mark.asyncio + async def test_clean_without_to_when_dry_run_is_true( self, config: PipelineConfig, handlers: ComponentHandlers, @@ -391,6 +398,6 @@ def test_clean_without_to_when_dry_run_is_true( assert connector.handlers.connector_handler - connector.clean(dry_run=True) + await connector.clean(dry_run=True) dry_run_handler.print_helm_diff.assert_called_once() diff --git a/tests/components/test_kubernetes_app.py b/tests/components/test_kubernetes_app.py index 81daee61f..81e1d8d2f 100644 --- a/tests/components/test_kubernetes_app.py +++ b/tests/components/test_kubernetes_app.py @@ -1,5 +1,5 @@ from pathlib import Path -from unittest.mock import MagicMock +from unittest.mock import AsyncMock, MagicMock import pytest from pytest_mock import MockerFixture @@ -37,9 +37,9 @@ def config(self) -> PipelineConfig: @pytest.fixture def handlers(self) -> ComponentHandlers: return ComponentHandlers( - schema_handler=MagicMock(), - connector_handler=MagicMock(), - topic_handler=MagicMock(), + schema_handler=AsyncMock(), + connector_handler=AsyncMock(), + topic_handler=AsyncMock(), ) @pytest.fixture @@ -157,7 +157,8 @@ async def test_should_raise_not_implemented_error_when_helm_chart_is_not_set( == str(error.value) ) - def test_should_call_helm_uninstall_when_destroying_kubernetes_app( + @pytest.mark.asyncio + async def test_should_call_helm_uninstall_when_destroying_kubernetes_app( self, config: PipelineConfig, handlers: ComponentHandlers, @@ -176,7 +177,7 @@ def test_should_call_helm_uninstall_when_destroying_kubernetes_app( stdout = 'KubernetesAppComponent - release "test-kubernetes-apps" uninstalled' helm_mock.uninstall.return_value = stdout - kubernetes_app.destroy(True) + await kubernetes_app.destroy(True) helm_mock.uninstall.assert_called_once_with( "test-namespace", "test-kubernetes-apps", True diff --git a/tests/components/test_producer_app.py b/tests/components/test_producer_app.py index 523e4a75b..2bdfe528c 100644 --- a/tests/components/test_producer_app.py +++ b/tests/components/test_producer_app.py @@ -1,6 +1,6 @@ import logging from pathlib import Path -from unittest.mock import ANY, MagicMock, AsyncMock +from unittest.mock import ANY, AsyncMock import pytest from pytest_mock import MockerFixture @@ -147,20 +147,22 @@ async def test_deploy_order_when_dry_run_is_false( ), ] - def test_destroy( + @pytest.mark.asyncio + async def test_destroy( self, producer_app: ProducerApp, mocker: MockerFixture, ): mock_helm_uninstall = mocker.patch.object(producer_app.helm, "uninstall") - producer_app.destroy(dry_run=True) + await producer_app.destroy(dry_run=True) mock_helm_uninstall.assert_called_once_with( "test-namespace", self.PRODUCER_APP_NAME, True ) - def test_should_not_reset_producer_app( + @pytest.mark.asyncio + async def test_should_not_reset_producer_app( self, producer_app: ProducerApp, mocker: MockerFixture, @@ -178,7 +180,7 @@ def test_should_not_reset_producer_app( mock.attach_mock(mock_helm_uninstall, "helm_uninstall") mock.attach_mock(mock_helm_print_helm_diff, "print_helm_diff") - producer_app.clean(dry_run=True) + await producer_app.clean(dry_run=True) assert mock.mock_calls == [ mocker.call.helm_uninstall( @@ -207,7 +209,8 @@ def test_should_not_reset_producer_app( ), ] - def test_should_clean_producer_app_and_deploy_clean_up_job_and_delete_clean_up_with_dry_run_false( + @pytest.mark.asyncio + async def test_should_clean_producer_app_and_deploy_clean_up_job_and_delete_clean_up_with_dry_run_false( self, mocker: MockerFixture, producer_app: ProducerApp ): mock_helm_upgrade_install = mocker.patch.object( @@ -219,7 +222,7 @@ def test_should_clean_producer_app_and_deploy_clean_up_job_and_delete_clean_up_w mock.attach_mock(mock_helm_upgrade_install, "helm_upgrade_install") mock.attach_mock(mock_helm_uninstall, "helm_uninstall") - producer_app.clean(dry_run=False) + await producer_app.clean(dry_run=False) assert mock.mock_calls == [ mocker.call.helm_uninstall( diff --git a/tests/components/test_streams_app.py b/tests/components/test_streams_app.py index e8b311d01..05db6d4c8 100644 --- a/tests/components/test_streams_app.py +++ b/tests/components/test_streams_app.py @@ -1,5 +1,5 @@ from pathlib import Path -from unittest.mock import MagicMock, AsyncMock +from unittest.mock import AsyncMock import pytest from pytest_mock import MockerFixture @@ -343,16 +343,18 @@ async def test_deploy_order_when_dry_run_is_false( ), ] - def test_destroy(self, streams_app: StreamsApp, mocker: MockerFixture): + @pytest.mark.asyncio + async def test_destroy(self, streams_app: StreamsApp, mocker: MockerFixture): mock_helm_uninstall = mocker.patch.object(streams_app.helm, "uninstall") - streams_app.destroy(dry_run=True) + await streams_app.destroy(dry_run=True) mock_helm_uninstall.assert_called_once_with( "test-namespace", self.STREAMS_APP_NAME, True ) - def test_reset_when_dry_run_is_false( + @pytest.mark.asyncio + async def test_reset_when_dry_run_is_false( self, streams_app: StreamsApp, mocker: MockerFixture ): mock_helm_upgrade_install = mocker.patch.object( @@ -365,7 +367,7 @@ def test_reset_when_dry_run_is_false( mock.attach_mock(mock_helm_uninstall, "helm_uninstall") dry_run = False - streams_app.reset(dry_run=dry_run) + await streams_app.reset(dry_run=dry_run) assert mock.mock_calls == [ mocker.call.helm_uninstall( @@ -390,7 +392,8 @@ def test_reset_when_dry_run_is_false( ), ] - def test_should_clean_streams_app_and_deploy_clean_up_job_and_delete_clean_up( + @pytest.mark.asyncio + async def test_should_clean_streams_app_and_deploy_clean_up_job_and_delete_clean_up( self, streams_app: StreamsApp, mocker: MockerFixture, @@ -405,7 +408,7 @@ def test_should_clean_streams_app_and_deploy_clean_up_job_and_delete_clean_up( mock.attach_mock(mock_helm_uninstall, "helm_uninstall") dry_run = False - streams_app.clean(dry_run=dry_run) + await streams_app.clean(dry_run=dry_run) assert mock.mock_calls == [ mocker.call.helm_uninstall( From 4003d805eda283ae784e71b145ebe7c11ff7c982 Mon Sep 17 00:00:00 2001 From: Alejandro Jaramillo Date: Mon, 31 Jul 2023 17:06:46 +0200 Subject: [PATCH 08/23] Add httpx as a global instance --- .../kafka_connect/connect_wrapper.py | 31 +++++------- .../component_handlers/topic/proxy_wrapper.py | 47 ++++++++----------- .../kafka_connect/test_connect_wrapper.py | 12 ++--- .../topic/test_proxy_wrapper.py | 12 ++--- 4 files changed, 43 insertions(+), 59 deletions(-) diff --git a/kpops/component_handlers/kafka_connect/connect_wrapper.py b/kpops/component_handlers/kafka_connect/connect_wrapper.py index dcea23b76..72d10ab96 100644 --- a/kpops/component_handlers/kafka_connect/connect_wrapper.py +++ b/kpops/component_handlers/kafka_connect/connect_wrapper.py @@ -31,6 +31,7 @@ def __init__(self, host: str | None): ) log.error(error_message) raise RuntimeError(error_message) + self._client = httpx.AsyncClient(base_url=host) self._host: str = host @property @@ -50,11 +51,9 @@ async def create_connector( config_json = kafka_connect_config.dict(exclude_none=True) connect_data = {"name": connector_name, "config": config_json} - client = httpx.AsyncClient() - response = await client.post( - url=f"{self._host}/connectors", headers=HEADERS, json=connect_data + response = await self._client.post( + url=f"/connectors", headers=HEADERS, json=connect_data ) - await client.aclose() if response.status_code == httpx.codes.CREATED: log.info(f"Connector {connector_name} created.") log.debug(response.json()) @@ -74,11 +73,9 @@ async def get_connector(self, connector_name: str) -> KafkaConnectResponse: :param connector_name: Nameof the crated connector :return: Information about the connector """ - client = httpx.AsyncClient() - response = await client.get( - url=f"{self._host}/connectors/{connector_name}", headers=HEADERS + response = await self._client.get( + url=f"/connectors/{connector_name}", headers=HEADERS ) - await client.aclose() if response.status_code == httpx.codes.OK: log.info(f"Connector {connector_name} exists.") log.debug(response.json()) @@ -104,13 +101,11 @@ async def update_connector_config( :return: Information about the connector after the change has been made. """ config_json = kafka_connect_config.dict(exclude_none=True) - client = httpx.AsyncClient() - response = await client.put( - url=f"{self._host}/connectors/{connector_name}/config", + response = await self._client.put( + url=f"/connectors/{connector_name}/config", headers=HEADERS, json=config_json, ) - await client.aclose() data: dict = response.json() if response.status_code == httpx.codes.OK: @@ -151,14 +146,12 @@ async def validate_connector_config( config_json = self.get_connector_config(connector_name, kafka_connect_config) connector_class = ConnectWrapper.get_connector_class_name(config_json) - client = httpx.AsyncClient() - response = await client.put( - url=f"{self._host}/connector-plugins/{connector_class}/config/validate", + response = await self._client.put( + url=f"/connector-plugins/{connector_class}/config/validate", headers=HEADERS, json=config_json, ) - await client.aclose() if response.status_code == httpx.codes.OK: kafka_connect_error_response = KafkaConnectConfigErrorResponse( @@ -181,11 +174,9 @@ async def delete_connector(self, connector_name: str) -> None: Deletes a connector, halting all tasks and deleting its configuration. API Reference:https://docs.confluent.io/platform/current/connect/references/restapi.html#delete--connectors-(string-name)- """ - client = httpx.AsyncClient() - response = await client.delete( - url=f"{self._host}/connectors/{connector_name}", headers=HEADERS + response = await self._client.delete( + url=f"/connectors/{connector_name}", headers=HEADERS ) - await client.aclose() if response.status_code == httpx.codes.NO_CONTENT: log.info(f"Connector {connector_name} deleted.") return diff --git a/kpops/component_handlers/topic/proxy_wrapper.py b/kpops/component_handlers/topic/proxy_wrapper.py index e56833ffa..67c6b1ce8 100644 --- a/kpops/component_handlers/topic/proxy_wrapper.py +++ b/kpops/component_handlers/topic/proxy_wrapper.py @@ -25,14 +25,17 @@ class ProxyWrapper: Wraps Kafka REST Proxy APIs """ + def __init__(self, pipeline_config: PipelineConfig) -> None: if not pipeline_config.kafka_rest_host: raise ValueError( "The Kafka REST Proxy host is not set. Please set the host in the config.yaml using the kafka_rest_host property or set the environemt variable KPOPS_REST_PROXY_HOST." ) - + self._client = httpx.AsyncClient(base_url=pipeline_config.kafka_rest_host) self._host = pipeline_config.kafka_rest_host + + @async_cached_property async def cluster_id(self) -> str: """ @@ -44,9 +47,8 @@ async def cluster_id(self) -> str: bootstrap.servers configuration. Therefore, only one Kafka cluster will be returned. :return: The Kafka cluster ID. """ - client = httpx.AsyncClient() - response = await client.get(url=f"{self._host}/v3/clusters") - await client.aclose() + response = await self._client.get(url=f"{self._host}/v3/clusters") + if response.status_code == httpx.codes.OK: cluster_information = response.json() return cluster_information["data"][0]["cluster_id"] @@ -63,13 +65,12 @@ async def create_topic(self, topic_spec: TopicSpec) -> None: API Reference: https://docs.confluent.io/platform/current/kafka-rest/api.html#post--clusters-cluster_id-topics :param topic_spec: The topic specification. """ - client = httpx.AsyncClient() - response = await client.post( - url=f"{self._host}/v3/clusters/{self.cluster_id}/topics", + response = await self._client.post( + url=f"/v3/clusters/{self.cluster_id}/topics", headers=HEADERS, json=topic_spec.dict(exclude_none=True), ) - await client.aclose() + if response.status_code == httpx.codes.CREATED: log.info(f"Topic {topic_spec.topic_name} created.") log.debug(response.json()) @@ -83,12 +84,11 @@ async def delete_topic(self, topic_name: str) -> None: API Reference: https://docs.confluent.io/platform/current/kafka-rest/api.html#delete--clusters-cluster_id-topics-topic_name :param topic_name: Name of the topic """ - client = httpx.AsyncClient() - response = await client.delete( - url=f"{self.host}/v3/clusters/{self.cluster_id}/topics/{topic_name}", + response = await self._client.delete( + url=f"/v3/clusters/{self.cluster_id}/topics/{topic_name}", headers=HEADERS, ) - await client.aclose() + if response.status_code == httpx.codes.NO_CONTENT: log.info(f"Topic {topic_name} deleted.") return @@ -103,9 +103,8 @@ async def get_topic(self, topic_name: str) -> TopicResponse: :return: Response of the get topic API """ - client = httpx.AsyncClient() - response = await client.get( - url=f"{self.host}/v3/clusters/{self.cluster_id}/topics/{topic_name}", + response = await self._client.get( + url=f"/v3/clusters/{self.cluster_id}/topics/{topic_name}", headers=HEADERS, ) @@ -132,12 +131,10 @@ async def get_topic_config(self, topic_name: str) -> TopicConfigResponse: :return: The topic configuration. """ - client = httpx.AsyncClient() - response = await client.get( - url=f"{self.host}/v3/clusters/{self.cluster_id}/topics/{topic_name}/configs", + response = await self._client.get( + url=f"/v3/clusters/{self.cluster_id}/topics/{topic_name}/configs", headers=HEADERS, ) - await client.aclose() if response.status_code == httpx.codes.OK: log.debug(f"Configs for {topic_name} found.") @@ -163,13 +160,11 @@ async def batch_alter_topic_config( :param topic_name: The topic name. :param config_name: The configuration parameter name. """ - client = httpx.AsyncClient() - response = await client.post( - url=f"{self.host}/v3/clusters/{self.cluster_id}/topics/{topic_name}/configs:alter", + response = await self._client.post( + url=f"/v3/clusters/{self.cluster_id}/topics/{topic_name}/configs:alter", headers=HEADERS, json={"data": json_body}, ) - await client.aclose() if response.status_code == httpx.codes.NO_CONTENT: log.info(f"Config of topic {topic_name} was altered.") @@ -183,12 +178,10 @@ async def get_broker_config(self) -> BrokerConfigResponse: API Reference: https://docs.confluent.io/platform/current/kafka-rest/api.html#get--clusters-cluster_id-brokers---configs :return: The broker configuration. """ - client = httpx.AsyncClient() - response = await client.get( - url=f"{self.host}/v3/clusters/{self.cluster_id}/brokers/-/configs", + response = await self._client.get( + url=f"/v3/clusters/{self.cluster_id}/brokers/-/configs", headers=HEADERS, ) - await client.aclose() if response.status_code == httpx.codes.OK: log.debug("Broker configs found.") diff --git a/tests/component_handlers/kafka_connect/test_connect_wrapper.py b/tests/component_handlers/kafka_connect/test_connect_wrapper.py index 5e4f5087f..95c9d6299 100644 --- a/tests/component_handlers/kafka_connect/test_connect_wrapper.py +++ b/tests/component_handlers/kafka_connect/test_connect_wrapper.py @@ -69,7 +69,7 @@ async def test_should_create_post_requests_for_given_connector_configuration( ) mock_post.assert_called_with( - url=f"{HOST}/connectors", + url=f"/connectors", headers=HEADERS, json={ "name": "test-connector", @@ -147,7 +147,7 @@ async def test_should_create_correct_get_connector_request( await self.connect_wrapper.get_connector(connector_name) mock_get.assert_called_with( - url=f"{HOST}/connectors/{connector_name}", + url=f"/connectors/{connector_name}", headers={"Accept": "application/json", "Content-Type": "application/json"}, ) @@ -277,7 +277,7 @@ async def test_should_create_correct_update_connector_request( ) mock_put.assert_called_with( - url=f"{HOST}/connectors/{connector_name}/config", + url=f"/connectors/{connector_name}/config", headers={"Accept": "application/json", "Content-Type": "application/json"}, json=KafkaConnectConfig(**configs).dict(), ) @@ -399,7 +399,7 @@ async def test_should_create_correct_delete_connector_request( await self.connect_wrapper.delete_connector(connector_name) mock_delete.assert_called_with( - url=f"{HOST}/connectors/{connector_name}", + url=f"/connectors/{connector_name}", headers=HEADERS, ) @@ -505,7 +505,7 @@ async def test_should_create_correct_validate_connector_config_request( ) mock_put.assert_called_with( - url=f"{HOST}/connector-plugins/FileStreamSinkConnector/config/validate", + url=f"/connector-plugins/FileStreamSinkConnector/config/validate", headers={"Accept": "application/json", "Content-Type": "application/json"}, json=KafkaConnectConfig(**configs).dict(), ) @@ -528,7 +528,7 @@ async def test_should_create_correct_validate_connector_config_and_name_gets_add ) mock_put.assert_called_with( - url=f"{HOST}/connector-plugins/{connector_name}/config/validate", + url=f"/connector-plugins/{connector_name}/config/validate", headers={"Accept": "application/json", "Content-Type": "application/json"}, json=KafkaConnectConfig(**{"name": connector_name, **configs}).dict(), ) diff --git a/tests/component_handlers/topic/test_proxy_wrapper.py b/tests/component_handlers/topic/test_proxy_wrapper.py index 34edbd74f..925e4a5b1 100644 --- a/tests/component_handlers/topic/test_proxy_wrapper.py +++ b/tests/component_handlers/topic/test_proxy_wrapper.py @@ -81,7 +81,7 @@ async def test_should_create_topic_with_all_topic_configuration( await self.proxy_wrapper.create_topic(topic_spec=TopicSpec(**topic_spec)) mock_post.assert_called_with( - url=f"{HOST}/v3/clusters/{self.proxy_wrapper.cluster_id}/topics", + url=f"/v3/clusters/{self.proxy_wrapper.cluster_id}/topics", headers=HEADERS, json=topic_spec, ) @@ -97,7 +97,7 @@ async def test_should_create_topic_with_no_configuration( await self.proxy_wrapper.create_topic(topic_spec=TopicSpec(**topic_spec)) mock_post.assert_called_with( - url=f"{HOST}/v3/clusters/{self.proxy_wrapper.cluster_id}/topics", + url=f"/v3/clusters/{self.proxy_wrapper.cluster_id}/topics", headers=HEADERS, json=topic_spec, ) @@ -111,7 +111,7 @@ async def test_should_call_get_topic(self, mock_get: MagicMock): await self.proxy_wrapper.get_topic(topic_name=topic_name) mock_get.assert_called_with( - url=f"{HOST}/v3/clusters/{self.proxy_wrapper.cluster_id}/topics/{topic_name}", + url=f"/v3/clusters/{self.proxy_wrapper.cluster_id}/topics/{topic_name}", headers=HEADERS, ) @@ -130,7 +130,7 @@ async def test_should_call_batch_alter_topic_config(self, mock_put: MagicMock): ) mock_put.assert_called_with( - url=f"{HOST}/v3/clusters/cluster-1/topics/{topic_name}/configs:alter", + url=f"/v3/clusters/cluster-1/topics/{topic_name}/configs:alter", headers=HEADERS, json={ "data": [ @@ -149,7 +149,7 @@ async def test_should_call_delete_topic(self, mock_delete: MagicMock): await self.proxy_wrapper.delete_topic(topic_name=topic_name) mock_delete.assert_called_with( - url=f"{HOST}/v3/clusters/{self.proxy_wrapper.cluster_id}/topics/{topic_name}", + url=f"/v3/clusters/{self.proxy_wrapper.cluster_id}/topics/{topic_name}", headers=HEADERS, ) @@ -160,7 +160,7 @@ async def test_should_call_get_broker_config(self, mock_get: MagicMock): await self.proxy_wrapper.get_broker_config() mock_get.assert_called_with( - url=f"{HOST}/v3/clusters/{self.proxy_wrapper.cluster_id}/brokers/-/configs", + url=f"/v3/clusters/{self.proxy_wrapper.cluster_id}/brokers/-/configs", headers=HEADERS, ) From 7fffe38f0530ccab299b432bf52a59634771a07d Mon Sep 17 00:00:00 2001 From: Alejandro Jaramillo Date: Mon, 31 Jul 2023 17:35:00 +0200 Subject: [PATCH 09/23] Fix linting --- kpops/component_handlers/kafka_connect/connect_wrapper.py | 2 +- kpops/component_handlers/topic/proxy_wrapper.py | 3 --- .../component_handlers/kafka_connect/test_connect_wrapper.py | 4 ++-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/kpops/component_handlers/kafka_connect/connect_wrapper.py b/kpops/component_handlers/kafka_connect/connect_wrapper.py index 72d10ab96..69d67d887 100644 --- a/kpops/component_handlers/kafka_connect/connect_wrapper.py +++ b/kpops/component_handlers/kafka_connect/connect_wrapper.py @@ -52,7 +52,7 @@ async def create_connector( connect_data = {"name": connector_name, "config": config_json} response = await self._client.post( - url=f"/connectors", headers=HEADERS, json=connect_data + url="/connectors", headers=HEADERS, json=connect_data ) if response.status_code == httpx.codes.CREATED: log.info(f"Connector {connector_name} created.") diff --git a/kpops/component_handlers/topic/proxy_wrapper.py b/kpops/component_handlers/topic/proxy_wrapper.py index 67c6b1ce8..2d9e18174 100644 --- a/kpops/component_handlers/topic/proxy_wrapper.py +++ b/kpops/component_handlers/topic/proxy_wrapper.py @@ -25,7 +25,6 @@ class ProxyWrapper: Wraps Kafka REST Proxy APIs """ - def __init__(self, pipeline_config: PipelineConfig) -> None: if not pipeline_config.kafka_rest_host: raise ValueError( @@ -34,8 +33,6 @@ def __init__(self, pipeline_config: PipelineConfig) -> None: self._client = httpx.AsyncClient(base_url=pipeline_config.kafka_rest_host) self._host = pipeline_config.kafka_rest_host - - @async_cached_property async def cluster_id(self) -> str: """ diff --git a/tests/component_handlers/kafka_connect/test_connect_wrapper.py b/tests/component_handlers/kafka_connect/test_connect_wrapper.py index 95c9d6299..7b88cea6c 100644 --- a/tests/component_handlers/kafka_connect/test_connect_wrapper.py +++ b/tests/component_handlers/kafka_connect/test_connect_wrapper.py @@ -69,7 +69,7 @@ async def test_should_create_post_requests_for_given_connector_configuration( ) mock_post.assert_called_with( - url=f"/connectors", + url="/connectors", headers=HEADERS, json={ "name": "test-connector", @@ -505,7 +505,7 @@ async def test_should_create_correct_validate_connector_config_request( ) mock_put.assert_called_with( - url=f"/connector-plugins/FileStreamSinkConnector/config/validate", + url="/connector-plugins/FileStreamSinkConnector/config/validate", headers={"Accept": "application/json", "Content-Type": "application/json"}, json=KafkaConnectConfig(**configs).dict(), ) From c222589699f224dc9070ce51220fd56a8a2c7f6d Mon Sep 17 00:00:00 2001 From: Alejandro Jaramillo Date: Mon, 31 Jul 2023 17:38:55 +0200 Subject: [PATCH 10/23] Delete f string --- kpops/component_handlers/topic/proxy_wrapper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kpops/component_handlers/topic/proxy_wrapper.py b/kpops/component_handlers/topic/proxy_wrapper.py index 2d9e18174..43e05840c 100644 --- a/kpops/component_handlers/topic/proxy_wrapper.py +++ b/kpops/component_handlers/topic/proxy_wrapper.py @@ -44,7 +44,7 @@ async def cluster_id(self) -> str: bootstrap.servers configuration. Therefore, only one Kafka cluster will be returned. :return: The Kafka cluster ID. """ - response = await self._client.get(url=f"{self._host}/v3/clusters") + response = await self._client.get(url="/v3/clusters") if response.status_code == httpx.codes.OK: cluster_information = response.json() From a14e39c5a39fe67643340337fcc57607daa73ef2 Mon Sep 17 00:00:00 2001 From: Alejandro Jaramillo Date: Mon, 31 Jul 2023 18:16:59 +0200 Subject: [PATCH 11/23] Add some logs --- kpops/component_handlers/topic/proxy_wrapper.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kpops/component_handlers/topic/proxy_wrapper.py b/kpops/component_handlers/topic/proxy_wrapper.py index 43e05840c..f863e237b 100644 --- a/kpops/component_handlers/topic/proxy_wrapper.py +++ b/kpops/component_handlers/topic/proxy_wrapper.py @@ -62,6 +62,8 @@ async def create_topic(self, topic_spec: TopicSpec) -> None: API Reference: https://docs.confluent.io/platform/current/kafka-rest/api.html#post--clusters-cluster_id-topics :param topic_spec: The topic specification. """ + print("This is the cluster id") + print(self.cluster_id) response = await self._client.post( url=f"/v3/clusters/{self.cluster_id}/topics", headers=HEADERS, From 2f7212fa00660b4219cb4532c734ac220b330bdd Mon Sep 17 00:00:00 2001 From: Alejandro Jaramillo Date: Mon, 31 Jul 2023 18:31:19 +0200 Subject: [PATCH 12/23] Add ignore to the await calls --- kpops/component_handlers/topic/proxy_wrapper.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/kpops/component_handlers/topic/proxy_wrapper.py b/kpops/component_handlers/topic/proxy_wrapper.py index f863e237b..51c3a1013 100644 --- a/kpops/component_handlers/topic/proxy_wrapper.py +++ b/kpops/component_handlers/topic/proxy_wrapper.py @@ -84,7 +84,7 @@ async def delete_topic(self, topic_name: str) -> None: :param topic_name: Name of the topic """ response = await self._client.delete( - url=f"/v3/clusters/{self.cluster_id}/topics/{topic_name}", + url=f"/v3/clusters/{await self.cluster_id}/topics/{topic_name}", # type: ignore headers=HEADERS, ) @@ -103,7 +103,7 @@ async def get_topic(self, topic_name: str) -> TopicResponse: """ response = await self._client.get( - url=f"/v3/clusters/{self.cluster_id}/topics/{topic_name}", + url=f"/v3/clusters/{await self.cluster_id}/topics/{topic_name}", # type: ignore headers=HEADERS, ) @@ -131,7 +131,7 @@ async def get_topic_config(self, topic_name: str) -> TopicConfigResponse: """ response = await self._client.get( - url=f"/v3/clusters/{self.cluster_id}/topics/{topic_name}/configs", + url=f"/v3/clusters/{await self.cluster_id}/topics/{topic_name}/configs", # type: ignore headers=HEADERS, ) @@ -160,7 +160,7 @@ async def batch_alter_topic_config( :param config_name: The configuration parameter name. """ response = await self._client.post( - url=f"/v3/clusters/{self.cluster_id}/topics/{topic_name}/configs:alter", + url=f"/v3/clusters/{await self.cluster_id}/topics/{topic_name}/configs:alter", # type: ignore headers=HEADERS, json={"data": json_body}, ) @@ -178,7 +178,7 @@ async def get_broker_config(self) -> BrokerConfigResponse: :return: The broker configuration. """ response = await self._client.get( - url=f"/v3/clusters/{self.cluster_id}/brokers/-/configs", + url=f"/v3/clusters/{await self.cluster_id}/brokers/-/configs", # type: ignore headers=HEADERS, ) From 5c333434dd68670379aebb8abc8eae7e463010cb Mon Sep 17 00:00:00 2001 From: Alejandro Jaramillo Date: Mon, 31 Jul 2023 18:34:46 +0200 Subject: [PATCH 13/23] Add issue --- kpops/component_handlers/topic/proxy_wrapper.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/kpops/component_handlers/topic/proxy_wrapper.py b/kpops/component_handlers/topic/proxy_wrapper.py index 51c3a1013..fd615ecf0 100644 --- a/kpops/component_handlers/topic/proxy_wrapper.py +++ b/kpops/component_handlers/topic/proxy_wrapper.py @@ -84,7 +84,7 @@ async def delete_topic(self, topic_name: str) -> None: :param topic_name: Name of the topic """ response = await self._client.delete( - url=f"/v3/clusters/{await self.cluster_id}/topics/{topic_name}", # type: ignore + url=f"/v3/clusters/{await self.cluster_id}/topics/{topic_name}", # type: ignore # FIXME https://github.com/ryananguiano/async_property/issues/17 headers=HEADERS, ) @@ -103,7 +103,7 @@ async def get_topic(self, topic_name: str) -> TopicResponse: """ response = await self._client.get( - url=f"/v3/clusters/{await self.cluster_id}/topics/{topic_name}", # type: ignore + url=f"/v3/clusters/{await self.cluster_id}/topics/{topic_name}", # type: ignore # FIXME https://github.com/ryananguiano/async_property/issues/17 headers=HEADERS, ) @@ -131,7 +131,7 @@ async def get_topic_config(self, topic_name: str) -> TopicConfigResponse: """ response = await self._client.get( - url=f"/v3/clusters/{await self.cluster_id}/topics/{topic_name}/configs", # type: ignore + url=f"/v3/clusters/{await self.cluster_id}/topics/{topic_name}/configs", # type: ignore # FIXME https://github.com/ryananguiano/async_property/issues/17 headers=HEADERS, ) @@ -160,7 +160,7 @@ async def batch_alter_topic_config( :param config_name: The configuration parameter name. """ response = await self._client.post( - url=f"/v3/clusters/{await self.cluster_id}/topics/{topic_name}/configs:alter", # type: ignore + url=f"/v3/clusters/{await self.cluster_id}/topics/{topic_name}/configs:alter", # type: ignore # FIXME https://github.com/ryananguiano/async_property/issues/17 headers=HEADERS, json={"data": json_body}, ) @@ -178,7 +178,7 @@ async def get_broker_config(self) -> BrokerConfigResponse: :return: The broker configuration. """ response = await self._client.get( - url=f"/v3/clusters/{await self.cluster_id}/brokers/-/configs", # type: ignore + url=f"/v3/clusters/{await self.cluster_id}/brokers/-/configs", # type: ignore # FIXME https://github.com/ryananguiano/async_property/issues/17 headers=HEADERS, ) From b4ffe8161c178c5c917c5cd83cb52ed486567b8c Mon Sep 17 00:00:00 2001 From: Alejandro Jaramillo Date: Mon, 31 Jul 2023 18:44:16 +0200 Subject: [PATCH 14/23] Remove prints --- kpops/component_handlers/topic/proxy_wrapper.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/kpops/component_handlers/topic/proxy_wrapper.py b/kpops/component_handlers/topic/proxy_wrapper.py index fd615ecf0..e10608d47 100644 --- a/kpops/component_handlers/topic/proxy_wrapper.py +++ b/kpops/component_handlers/topic/proxy_wrapper.py @@ -62,8 +62,6 @@ async def create_topic(self, topic_spec: TopicSpec) -> None: API Reference: https://docs.confluent.io/platform/current/kafka-rest/api.html#post--clusters-cluster_id-topics :param topic_spec: The topic specification. """ - print("This is the cluster id") - print(self.cluster_id) response = await self._client.post( url=f"/v3/clusters/{self.cluster_id}/topics", headers=HEADERS, From 453294a1b98c0c6a6c2096df61746e13e131ac6b Mon Sep 17 00:00:00 2001 From: Alejandro Jaramillo Date: Tue, 1 Aug 2023 14:16:31 +0200 Subject: [PATCH 15/23] Implement comments --- .../component_handlers/topic/proxy_wrapper.py | 19 +++++---- kpops/utils/async_utils.py | 5 --- poetry.lock | 13 +----- pyproject.toml | 1 - .../kafka_connect/test_connect_handler.py | 41 +++++++------------ .../kafka_connect/test_connect_wrapper.py | 2 +- .../topic/test_proxy_wrapper.py | 8 ++-- 7 files changed, 30 insertions(+), 59 deletions(-) delete mode 100644 kpops/utils/async_utils.py diff --git a/kpops/component_handlers/topic/proxy_wrapper.py b/kpops/component_handlers/topic/proxy_wrapper.py index e10608d47..e4c40485a 100644 --- a/kpops/component_handlers/topic/proxy_wrapper.py +++ b/kpops/component_handlers/topic/proxy_wrapper.py @@ -1,7 +1,7 @@ import logging +from functools import cached_property import httpx -from async_property import async_cached_property from kpops.cli.pipeline_config import PipelineConfig from kpops.component_handlers.topic.exception import ( @@ -31,10 +31,11 @@ def __init__(self, pipeline_config: PipelineConfig) -> None: "The Kafka REST Proxy host is not set. Please set the host in the config.yaml using the kafka_rest_host property or set the environemt variable KPOPS_REST_PROXY_HOST." ) self._client = httpx.AsyncClient(base_url=pipeline_config.kafka_rest_host) + self._sync_client = httpx.Client(base_url=pipeline_config.kafka_rest_host) self._host = pipeline_config.kafka_rest_host - @async_cached_property - async def cluster_id(self) -> str: + @cached_property + def cluster_id(self) -> str: """ Gets the Kafka cluster ID by sending a requests to Kafka REST proxy. More information about the cluster ID can be found here: @@ -44,7 +45,7 @@ async def cluster_id(self) -> str: bootstrap.servers configuration. Therefore, only one Kafka cluster will be returned. :return: The Kafka cluster ID. """ - response = await self._client.get(url="/v3/clusters") + response = self._sync_client.get(url="/v3/clusters") if response.status_code == httpx.codes.OK: cluster_information = response.json() @@ -82,7 +83,7 @@ async def delete_topic(self, topic_name: str) -> None: :param topic_name: Name of the topic """ response = await self._client.delete( - url=f"/v3/clusters/{await self.cluster_id}/topics/{topic_name}", # type: ignore # FIXME https://github.com/ryananguiano/async_property/issues/17 + url=f"/v3/clusters/{self.cluster_id}/topics/{topic_name}", # type: ignore # FIXME https://github.com/ryananguiano/async_property/issues/17 headers=HEADERS, ) @@ -101,7 +102,7 @@ async def get_topic(self, topic_name: str) -> TopicResponse: """ response = await self._client.get( - url=f"/v3/clusters/{await self.cluster_id}/topics/{topic_name}", # type: ignore # FIXME https://github.com/ryananguiano/async_property/issues/17 + url=f"/v3/clusters/{self.cluster_id}/topics/{topic_name}", # type: ignore # FIXME https://github.com/ryananguiano/async_property/issues/17 headers=HEADERS, ) @@ -129,7 +130,7 @@ async def get_topic_config(self, topic_name: str) -> TopicConfigResponse: """ response = await self._client.get( - url=f"/v3/clusters/{await self.cluster_id}/topics/{topic_name}/configs", # type: ignore # FIXME https://github.com/ryananguiano/async_property/issues/17 + url=f"/v3/clusters/{self.cluster_id}/topics/{topic_name}/configs", # type: ignore # FIXME https://github.com/ryananguiano/async_property/issues/17 headers=HEADERS, ) @@ -158,7 +159,7 @@ async def batch_alter_topic_config( :param config_name: The configuration parameter name. """ response = await self._client.post( - url=f"/v3/clusters/{await self.cluster_id}/topics/{topic_name}/configs:alter", # type: ignore # FIXME https://github.com/ryananguiano/async_property/issues/17 + url=f"/v3/clusters/{self.cluster_id}/topics/{topic_name}/configs:alter", # type: ignore # FIXME https://github.com/ryananguiano/async_property/issues/17 headers=HEADERS, json={"data": json_body}, ) @@ -176,7 +177,7 @@ async def get_broker_config(self) -> BrokerConfigResponse: :return: The broker configuration. """ response = await self._client.get( - url=f"/v3/clusters/{await self.cluster_id}/brokers/-/configs", # type: ignore # FIXME https://github.com/ryananguiano/async_property/issues/17 + url=f"/v3/clusters/{self.cluster_id}/brokers/-/configs", # type: ignore # FIXME https://github.com/ryananguiano/async_property/issues/17 headers=HEADERS, ) diff --git a/kpops/utils/async_utils.py b/kpops/utils/async_utils.py deleted file mode 100644 index 2b71286bb..000000000 --- a/kpops/utils/async_utils.py +++ /dev/null @@ -1,5 +0,0 @@ -import asyncio - - -def get_loop(): - return asyncio.get_running_loop() diff --git a/poetry.lock b/poetry.lock index c8b37298b..cd9fe5157 100644 --- a/poetry.lock +++ b/poetry.lock @@ -31,17 +31,6 @@ doc = ["packaging", "sphinx-autodoc-typehints (>=1.2.0)", "sphinx-rtd-theme"] test = ["contextlib2", "coverage[toml] (>=4.5)", "hypothesis (>=4.0)", "mock (>=4)", "pytest (>=7.0)", "pytest-mock (>=3.6.1)", "trustme", "uvloop (<0.15)", "uvloop (>=0.15)"] trio = ["trio (>=0.16,<0.22)"] -[[package]] -name = "async-property" -version = "0.2.2" -description = "Python decorator for async properties." -optional = false -python-versions = "*" -files = [ - {file = "async_property-0.2.2-py2.py3-none-any.whl", hash = "sha256:8924d792b5843994537f8ed411165700b27b2bd966cefc4daeefc1253442a9d7"}, - {file = "async_property-0.2.2.tar.gz", hash = "sha256:17d9bd6ca67e27915a75d92549df64b5c7174e9dc806b30a3934dc4ff0506380"}, -] - [[package]] name = "attrs" version = "22.1.0" @@ -1613,4 +1602,4 @@ watchmedo = ["PyYAML (>=3.10)"] [metadata] lock-version = "2.0" python-versions = "^3.10" -content-hash = "9f0261bfb360752cf70161de6284b9b3b08a54569c4ee862b35c02e946e34f28" +content-hash = "e58fdd8b8e2557f7612d13c6241dbce1e8af43222e93d18d869c898d7fcd760c" diff --git a/pyproject.toml b/pyproject.toml index 416462896..502377d91 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -37,7 +37,6 @@ cachetools = "^5.2.0" dictdiffer = "^0.9.0" python-schema-registry-client = "^2.4.1" httpx = "^0.24.1" -async-property = "^0.2.2" [tool.poetry.group.dev.dependencies] diff --git a/tests/component_handlers/kafka_connect/test_connect_handler.py b/tests/component_handlers/kafka_connect/test_connect_handler.py index 45103133f..9fba60a9a 100644 --- a/tests/component_handlers/kafka_connect/test_connect_handler.py +++ b/tests/component_handlers/kafka_connect/test_connect_handler.py @@ -40,6 +40,10 @@ def log_error_mock(self, mocker: MockerFixture) -> MagicMock: "kpops.component_handlers.kafka_connect.kafka_connect_handler.log.error" ) + @pytest.fixture + def connector_wrapper(self): + return AsyncMock() + @pytest.fixture def renderer_diff_mock(self, mocker: MockerFixture) -> MagicMock: return mocker.patch( @@ -58,8 +62,8 @@ async def test_should_create_connector_in_dry_run( self, renderer_diff_mock: MagicMock, log_info_mock: MagicMock, + connector_wrapper: AsyncMock, ): - connector_wrapper = AsyncMock() handler = self.connector_handler(connector_wrapper) renderer_diff_mock.return_value = None @@ -81,10 +85,8 @@ async def test_should_create_connector_in_dry_run( @pytest.mark.asyncio async def test_should_log_correct_message_when_create_connector_and_connector_not_exists_in_dry_run( - self, - log_info_mock: MagicMock, + self, log_info_mock: MagicMock, connector_wrapper: AsyncMock ): - connector_wrapper = AsyncMock() handler = self.connector_handler(connector_wrapper) connector_wrapper.get_connector.side_effect = ConnectorNotFoundException() @@ -112,10 +114,8 @@ async def test_should_log_correct_message_when_create_connector_and_connector_no @pytest.mark.asyncio async def test_should_log_correct_message_when_create_connector_and_connector_exists_in_dry_run( - self, - log_info_mock: MagicMock, + self, log_info_mock: MagicMock, connector_wrapper: AsyncMock ): - connector_wrapper = AsyncMock() handler = self.connector_handler(connector_wrapper) actual_response = { @@ -158,10 +158,8 @@ async def test_should_log_correct_message_when_create_connector_and_connector_ex @pytest.mark.asyncio async def test_should_log_invalid_config_when_create_connector_in_dry_run( - self, renderer_diff_mock: MagicMock + self, renderer_diff_mock: MagicMock, connector_wrapper: AsyncMock ): - connector_wrapper = AsyncMock() - errors = [ "Missing required configuration file which has no default value.", "Missing connector name.", @@ -186,9 +184,8 @@ async def test_should_log_invalid_config_when_create_connector_in_dry_run( @pytest.mark.asyncio async def test_should_call_update_connector_config_when_connector_exists_not_dry_run( - self, + self, connector_wrapper: AsyncMock ): - connector_wrapper = AsyncMock() handler = self.connector_handler(connector_wrapper) config = KafkaConnectConfig() @@ -201,10 +198,8 @@ async def test_should_call_update_connector_config_when_connector_exists_not_dry @pytest.mark.asyncio async def test_should_call_create_connector_when_connector_does_not_exists_not_dry_run( - self, + self, connector_wrapper: AsyncMock ): - connector_wrapper = AsyncMock() - handler = self.connector_handler(connector_wrapper) config = KafkaConnectConfig() @@ -217,11 +212,8 @@ async def test_should_call_create_connector_when_connector_does_not_exists_not_d @pytest.mark.asyncio async def test_should_print_correct_log_when_destroying_connector_in_dry_run( - self, - log_info_mock: MagicMock, + self, log_info_mock: MagicMock, connector_wrapper: AsyncMock ): - connector_wrapper = AsyncMock() - handler = self.connector_handler(connector_wrapper) await handler.destroy_connector(CONNECTOR_NAME, True) @@ -234,10 +226,8 @@ async def test_should_print_correct_log_when_destroying_connector_in_dry_run( @pytest.mark.asyncio async def test_should_print_correct_warning_log_when_destroying_connector_and_connector_exists_in_dry_run( - self, - log_warning_mock: MagicMock, + self, log_warning_mock: MagicMock, connector_wrapper: AsyncMock ): - connector_wrapper = AsyncMock() connector_wrapper.get_connector.side_effect = ConnectorNotFoundException() handler = self.connector_handler(connector_wrapper) @@ -250,9 +240,8 @@ async def test_should_print_correct_warning_log_when_destroying_connector_and_co @pytest.mark.asyncio async def test_should_call_delete_connector_when_destroying_existing_connector_not_dry_run( - self, + self, connector_wrapper: AsyncMock ): - connector_wrapper = AsyncMock() handler = self.connector_handler(connector_wrapper) await handler.destroy_connector(CONNECTOR_NAME, False) @@ -263,10 +252,8 @@ async def test_should_call_delete_connector_when_destroying_existing_connector_n @pytest.mark.asyncio async def test_should_print_correct_warning_log_when_destroying_connector_and_connector_exists_not_dry_run( - self, - log_warning_mock: MagicMock, + self, log_warning_mock: MagicMock, connector_wrapper: AsyncMock ): - connector_wrapper = AsyncMock() connector_wrapper.get_connector.side_effect = ConnectorNotFoundException() handler = self.connector_handler(connector_wrapper) diff --git a/tests/component_handlers/kafka_connect/test_connect_wrapper.py b/tests/component_handlers/kafka_connect/test_connect_wrapper.py index 7b88cea6c..6aab50364 100644 --- a/tests/component_handlers/kafka_connect/test_connect_wrapper.py +++ b/tests/component_handlers/kafka_connect/test_connect_wrapper.py @@ -79,7 +79,7 @@ async def test_should_create_post_requests_for_given_connector_configuration( @pytest.mark.asyncio async def test_should_return_correct_response_when_connector_created( - self, httpx_mock + self, httpx_mock: HTTPXMock ): actual_response = { "name": "hdfs-sink-connector", diff --git a/tests/component_handlers/topic/test_proxy_wrapper.py b/tests/component_handlers/topic/test_proxy_wrapper.py index 925e4a5b1..90eb5421a 100644 --- a/tests/component_handlers/topic/test_proxy_wrapper.py +++ b/tests/component_handlers/topic/test_proxy_wrapper.py @@ -1,7 +1,7 @@ import json from pathlib import Path from typing import Any -from unittest.mock import MagicMock, patch +from unittest.mock import AsyncMock, MagicMock, patch import pytest import pytest_asyncio @@ -49,7 +49,7 @@ async def setup(self, httpx_mock: HTTPXMock): status_code=200, ) assert self.proxy_wrapper.host == HOST - assert await self.proxy_wrapper.cluster_id == "cluster-1" # type: ignore + assert self.proxy_wrapper.cluster_id == "cluster-1" # type: ignore @pytest.mark.asyncio async def test_should_raise_exception_when_host_is_not_set(self): @@ -65,7 +65,7 @@ async def test_should_raise_exception_when_host_is_not_set(self): @pytest.mark.asyncio @patch("httpx.AsyncClient.post") async def test_should_create_topic_with_all_topic_configuration( - self, mock_post: MagicMock + self, mock_post: AsyncMock ): topic_spec = { "topic_name": "topic-X", @@ -89,7 +89,7 @@ async def test_should_create_topic_with_all_topic_configuration( @pytest.mark.asyncio @patch("httpx.AsyncClient.post") async def test_should_create_topic_with_no_configuration( - self, mock_post: MagicMock + self, mock_post: AsyncMock ): topic_spec: dict[str, Any] = {"topic_name": "topic-X"} From 167d60158d34680aaa0dbe344bc08d519b821308 Mon Sep 17 00:00:00 2001 From: Alejandro Jaramillo Date: Wed, 2 Aug 2023 09:48:07 +0200 Subject: [PATCH 16/23] Migrate SchemaHandler --- .../schema_handler/schema_handler.py | 50 +++++++++-------- kpops/components/base_components/kafka_app.py | 2 +- .../base_components/kafka_connector.py | 4 +- .../schema_handler/test_schema_handler.py | 54 +++++++++++-------- 4 files changed, 61 insertions(+), 49 deletions(-) diff --git a/kpops/component_handlers/schema_handler/schema_handler.py b/kpops/component_handlers/schema_handler/schema_handler.py index a053ccc62..3a408a519 100644 --- a/kpops/component_handlers/schema_handler/schema_handler.py +++ b/kpops/component_handlers/schema_handler/schema_handler.py @@ -4,7 +4,7 @@ import logging from functools import cached_property -from schema_registry.client import SchemaRegistryClient +from schema_registry.client import AsyncSchemaRegistryClient from schema_registry.client.schema import AvroSchema from kpops.cli.exception import ClassNotFoundError @@ -22,7 +22,7 @@ class SchemaHandler: def __init__(self, url: str, components_module: str | None): - self.schema_registry_client = SchemaRegistryClient(url) + self.schema_registry_client = AsyncSchemaRegistryClient(url) self.components_module = components_module @cached_property @@ -52,7 +52,7 @@ def load_schema_handler( components_module=components_module, ) - def submit_schemas(self, to_section: ToSection, dry_run: bool = True) -> None: + async def submit_schemas(self, to_section: ToSection, dry_run: bool = True) -> None: for topic_name, config in to_section.topics.items(): value_schema_class = config.value_schema key_schema_class = config.key_schema @@ -60,23 +60,25 @@ def submit_schemas(self, to_section: ToSection, dry_run: bool = True) -> None: schema = self.schema_provider.provide_schema( value_schema_class, to_section.models ) - self.__submit_value_schema( + await self.__submit_value_schema( schema, value_schema_class, dry_run, topic_name ) if key_schema_class is not None: schema = self.schema_provider.provide_schema( key_schema_class, to_section.models ) - self.__submit_key_schema(schema, key_schema_class, dry_run, topic_name) + await self.__submit_key_schema( + schema, key_schema_class, dry_run, topic_name + ) - def delete_schemas(self, to_section: ToSection, dry_run: bool = True) -> None: + async def delete_schemas(self, to_section: ToSection, dry_run: bool = True) -> None: for topic_name, config in to_section.topics.items(): if config.value_schema is not None: - self.__delete_subject(f"{topic_name}-value", dry_run) + await self.__delete_subject(f"{topic_name}-value", dry_run) if config.key_schema is not None: - self.__delete_subject(f"{topic_name}-key", dry_run) + await self.__delete_subject(f"{topic_name}-key", dry_run) - def __submit_key_schema( + async def __submit_key_schema( self, schema: Schema, schema_class: str, @@ -84,14 +86,14 @@ def __submit_key_schema( topic_name: str, ) -> None: subject = f"{topic_name}-key" - self.__submit_schema( + await self.__submit_schema( subject=subject, schema=schema, schema_class=schema_class, dry_run=dry_run, ) - def __submit_value_schema( + async def __submit_value_schema( self, schema: Schema, schema_class: str, @@ -99,14 +101,14 @@ def __submit_value_schema( topic_name: str, ) -> None: subject = f"{topic_name}-value" - self.__submit_schema( + await self.__submit_schema( subject=subject, schema=schema, schema_class=schema_class, dry_run=dry_run, ) - def __submit_schema( + async def __submit_schema( self, subject: str, schema: Schema, @@ -114,8 +116,8 @@ def __submit_schema( dry_run: bool, ): if dry_run: - if self.__subject_exists(subject): - self.__check_compatibility(schema, schema_class, subject) + if await self.__subject_exists(subject): + await self.__check_compatibility(schema, schema_class, subject) else: log.info( greenify( @@ -123,20 +125,22 @@ def __submit_schema( ) ) else: - self.schema_registry_client.register(subject=subject, schema=schema) + await self.schema_registry_client.register(subject=subject, schema=schema) log.info( f"Schema Submission: schema submitted for {subject} with model {schema_class}." ) - def __subject_exists(self, subject: str) -> bool: - return len(self.schema_registry_client.get_versions(subject)) > 0 + async def __subject_exists(self, subject: str) -> bool: + return len(await self.schema_registry_client.get_versions(subject)) > 0 - def __check_compatibility( + async def __check_compatibility( self, schema: Schema, schema_class: str, subject: str ) -> None: - registered_version = self.schema_registry_client.check_version(subject, schema) + registered_version = await self.schema_registry_client.check_version( + subject, schema + ) if registered_version is None: - if not self.schema_registry_client.test_compatibility( + if not await self.schema_registry_client.test_compatibility( subject=subject, schema=schema ): schema_str = ( @@ -156,11 +160,11 @@ def __check_compatibility( f"Schema Submission: compatible schema for {subject} with model {schema_class}." ) - def __delete_subject(self, subject: str, dry_run: bool) -> None: + async def __delete_subject(self, subject: str, dry_run: bool) -> None: if dry_run: log.info(magentaify(f"Schema Deletion: will delete subject {subject}.")) else: - version_list = self.schema_registry_client.delete_subject(subject) + version_list = await self.schema_registry_client.delete_subject(subject) log.info( f"Schema Deletion: deleted {len(version_list)} versions for subject {subject}." ) diff --git a/kpops/components/base_components/kafka_app.py b/kpops/components/base_components/kafka_app.py index 23ebec8dc..32f785cbd 100644 --- a/kpops/components/base_components/kafka_app.py +++ b/kpops/components/base_components/kafka_app.py @@ -112,7 +112,7 @@ async def deploy(self, dry_run: bool) -> None: ) if self.handlers.schema_handler: - self.handlers.schema_handler.submit_schemas( + await self.handlers.schema_handler.submit_schemas( to_section=self.to, dry_run=dry_run ) await super().deploy(dry_run) diff --git a/kpops/components/base_components/kafka_connector.py b/kpops/components/base_components/kafka_connector.py index 31d1a2afa..f30dbf762 100644 --- a/kpops/components/base_components/kafka_connector.py +++ b/kpops/components/base_components/kafka_connector.py @@ -127,7 +127,7 @@ async def deploy(self, dry_run: bool) -> None: ) if self.handlers.schema_handler: - self.handlers.schema_handler.submit_schemas( + await self.handlers.schema_handler.submit_schemas( to_section=self.to, dry_run=dry_run ) @@ -145,7 +145,7 @@ async def destroy(self, dry_run: bool) -> None: async def clean(self, dry_run: bool) -> None: if self.to: if self.handlers.schema_handler: - self.handlers.schema_handler.delete_schemas( + await self.handlers.schema_handler.delete_schemas( to_section=self.to, dry_run=dry_run ) await self.handlers.topic_handler.delete_topics(self.to, dry_run=dry_run) diff --git a/tests/component_handlers/schema_handler/test_schema_handler.py b/tests/component_handlers/schema_handler/test_schema_handler.py index ccea021c6..f7487eca2 100644 --- a/tests/component_handlers/schema_handler/test_schema_handler.py +++ b/tests/component_handlers/schema_handler/test_schema_handler.py @@ -1,7 +1,7 @@ import json from pathlib import Path from unittest import mock -from unittest.mock import MagicMock +from unittest.mock import AsyncMock, MagicMock import pytest from pydantic import BaseModel @@ -48,10 +48,11 @@ def find_class_mock(mocker: MockerFixture) -> MagicMock: @pytest.fixture(autouse=True) def schema_registry_mock(mocker: MockerFixture) -> MagicMock: - schema_registry_mock = mocker.patch( - "kpops.component_handlers.schema_handler.schema_handler.SchemaRegistryClient" + schema_registry_mock_constructor = mocker.patch( + "kpops.component_handlers.schema_handler.schema_handler.AsyncSchemaRegistryClient", ) - return schema_registry_mock.return_value + schema_registry_mock_constructor.return_value = AsyncMock() + return schema_registry_mock_constructor.return_value @pytest.fixture() @@ -156,7 +157,8 @@ def test_should_raise_value_error_when_schema_provider_is_called_and_components_ ) -def test_should_log_info_when_submit_schemas_that_not_exists_and_dry_run_true( +@pytest.mark.asyncio +async def test_should_log_info_when_submit_schemas_that_not_exists_and_dry_run_true( to_section: ToSection, log_info_mock: MagicMock, schema_registry_mock: MagicMock ): schema_handler = SchemaHandler( @@ -165,7 +167,7 @@ def test_should_log_info_when_submit_schemas_that_not_exists_and_dry_run_true( schema_registry_mock.get_versions.return_value = [] - schema_handler.submit_schemas(to_section, True) + await schema_handler.submit_schemas(to_section, True) log_info_mock.assert_called_once_with( greenify("Schema Submission: The subject topic-X-value will be submitted.") @@ -173,11 +175,12 @@ def test_should_log_info_when_submit_schemas_that_not_exists_and_dry_run_true( schema_registry_mock.register.assert_not_called() -def test_should_log_info_when_submit_schemas_that_exists_and_dry_run_true( +@pytest.mark.asyncio +async def test_should_log_info_when_submit_schemas_that_exists_and_dry_run_true( topic_config: TopicConfig, to_section: ToSection, log_info_mock: MagicMock, - schema_registry_mock: MagicMock, + schema_registry_mock: AsyncMock, ): schema_handler = SchemaHandler( url="http://mock:8081", components_module=TEST_SCHEMA_PROVIDER_MODULE @@ -187,7 +190,7 @@ def test_should_log_info_when_submit_schemas_that_exists_and_dry_run_true( schema_registry_mock.check_version.return_value = None schema_registry_mock.test_compatibility.return_value = True - schema_handler.submit_schemas(to_section, True) + await schema_handler.submit_schemas(to_section, True) log_info_mock.assert_called_once_with( f"Schema Submission: compatible schema for topic-X-value with model {topic_config.value_schema}." @@ -195,10 +198,11 @@ def test_should_log_info_when_submit_schemas_that_exists_and_dry_run_true( schema_registry_mock.register.assert_not_called() -def test_should_raise_exception_when_submit_schema_that_exists_and_not_compatible_and_dry_run_true( +@pytest.mark.asyncio +async def test_should_raise_exception_when_submit_schema_that_exists_and_not_compatible_and_dry_run_true( topic_config: TopicConfig, to_section: ToSection, - schema_registry_mock: MagicMock, + schema_registry_mock: AsyncMock, ): schema_provider = TestSchemaProvider() schema_handler = SchemaHandler( @@ -211,7 +215,7 @@ def test_should_raise_exception_when_submit_schema_that_exists_and_not_compatibl schema_registry_mock.test_compatibility.return_value = False with pytest.raises(Exception) as exception: - schema_handler.submit_schemas(to_section, True) + await schema_handler.submit_schemas(to_section, True) assert "Schema is not compatible for" in str(exception.value) EXPECTED_SCHEMA = { @@ -233,12 +237,13 @@ def test_should_raise_exception_when_submit_schema_that_exists_and_not_compatibl schema_registry_mock.register.assert_not_called() -def test_should_log_debug_when_submit_schema_that_exists_and_registered_under_version_and_dry_run_true( +@pytest.mark.asyncio +async def test_should_log_debug_when_submit_schema_that_exists_and_registered_under_version_and_dry_run_true( topic_config: TopicConfig, to_section: ToSection, log_info_mock: MagicMock, log_debug_mock: MagicMock, - schema_registry_mock: MagicMock, + schema_registry_mock: AsyncMock, ): schema_provider = TestSchemaProvider() schema_handler = SchemaHandler( @@ -251,7 +256,7 @@ def test_should_log_debug_when_submit_schema_that_exists_and_registered_under_ve schema_registry_mock.get_versions.return_value = [1] schema_registry_mock.check_version.return_value = registered_version - schema_handler.submit_schemas(to_section, True) + await schema_handler.submit_schemas(to_section, True) assert log_info_mock.mock_calls == [ mock.call( @@ -268,11 +273,12 @@ def test_should_log_debug_when_submit_schema_that_exists_and_registered_under_ve schema_registry_mock.register.assert_not_called() -def test_should_submit_non_existing_schema_when_not_dry( +@pytest.mark.asyncio +async def test_should_submit_non_existing_schema_when_not_dry( topic_config: TopicConfig, to_section: ToSection, log_info_mock: MagicMock, - schema_registry_mock: MagicMock, + schema_registry_mock: AsyncMock, ): schema_provider = TestSchemaProvider() schema_class = "com.bakdata.kpops.test.SchemaHandlerTest" @@ -283,7 +289,7 @@ def test_should_submit_non_existing_schema_when_not_dry( schema_registry_mock.get_versions.return_value = [] - schema_handler.submit_schemas(to_section, False) + await schema_handler.submit_schemas(to_section, False) subject = "topic-X-value" log_info_mock.assert_called_once_with( @@ -296,10 +302,11 @@ def test_should_submit_non_existing_schema_when_not_dry( ) -def test_should_log_correct_message_when_delete_schemas_and_in_dry_run( +@pytest.mark.asyncio +async def test_should_log_correct_message_when_delete_schemas_and_in_dry_run( to_section: ToSection, log_info_mock: MagicMock, - schema_registry_mock: MagicMock, + schema_registry_mock: AsyncMock, ): schema_handler = SchemaHandler( url="http://mock:8081", components_module=TEST_SCHEMA_PROVIDER_MODULE @@ -307,7 +314,7 @@ def test_should_log_correct_message_when_delete_schemas_and_in_dry_run( schema_registry_mock.get_versions.return_value = [] - schema_handler.delete_schemas(to_section, True) + await schema_handler.delete_schemas(to_section, True) log_info_mock.assert_called_once_with( magentaify("Schema Deletion: will delete subject topic-X-value.") @@ -316,7 +323,8 @@ def test_should_log_correct_message_when_delete_schemas_and_in_dry_run( schema_registry_mock.delete_subject.assert_not_called() -def test_should_delete_schemas_when_not_in_dry_run( +@pytest.mark.asyncio +async def test_should_delete_schemas_when_not_in_dry_run( to_section: ToSection, schema_registry_mock: MagicMock ): schema_handler = SchemaHandler( @@ -325,6 +333,6 @@ def test_should_delete_schemas_when_not_in_dry_run( schema_registry_mock.get_versions.return_value = [] - schema_handler.delete_schemas(to_section, False) + await schema_handler.delete_schemas(to_section, False) schema_registry_mock.delete_subject.assert_called_once_with("topic-X-value") From 29451507482b4f997b7dd1675aee0b6a752d63a7 Mon Sep 17 00:00:00 2001 From: Alejandro Jaramillo Date: Tue, 8 Aug 2023 11:18:46 +0200 Subject: [PATCH 17/23] Implement changes --- docs/docs/schema/pipeline.json | 4 ++-- .../component_handlers/topic/proxy_wrapper.py | 16 ++++++++------ .../snapshots/snap_test_schema_generation.py | 4 ++-- .../kafka_connect/test_connect_handler.py | 2 +- .../kafka_connect/test_connect_wrapper.py | 12 +++++----- .../schema_handler/test_schema_handler.py | 4 ++-- .../topic/test_proxy_wrapper.py | 22 +++++++++---------- 7 files changed, 33 insertions(+), 31 deletions(-) diff --git a/docs/docs/schema/pipeline.json b/docs/docs/schema/pipeline.json index 76da0f23c..3f71896a1 100644 --- a/docs/docs/schema/pipeline.json +++ b/docs/docs/schema/pipeline.json @@ -87,7 +87,7 @@ "type": "object" }, "InputTopicTypes": { - "description": "Input topic types\n\ninput (input topic), input_pattern (input pattern topic), extra (extra topic), extra_pattern (extra pattern topic).\nEvery extra topic must have a role.", + "description": "Input topic types\n\n input (input topic), input_pattern (input pattern topic), extra (extra topic), extra_pattern (extra pattern topic).\n Every extra topic must have a role.\n ", "enum": [ "input", "extra", @@ -641,7 +641,7 @@ "type": "object" }, "OutputTopicTypes": { - "description": "Types of output topic\n\nError (error topic), output (output topic), and extra topics. Every extra topic must have a role.", + "description": "Types of output topic\n\n Error (error topic), output (output topic), and extra topics. Every extra topic must have a role.\n ", "enum": [ "error", "output", diff --git a/kpops/component_handlers/topic/proxy_wrapper.py b/kpops/component_handlers/topic/proxy_wrapper.py index e4c40485a..aa96e7ff1 100644 --- a/kpops/component_handlers/topic/proxy_wrapper.py +++ b/kpops/component_handlers/topic/proxy_wrapper.py @@ -30,7 +30,9 @@ def __init__(self, pipeline_config: PipelineConfig) -> None: raise ValueError( "The Kafka REST Proxy host is not set. Please set the host in the config.yaml using the kafka_rest_host property or set the environemt variable KPOPS_REST_PROXY_HOST." ) - self._client = httpx.AsyncClient(base_url=pipeline_config.kafka_rest_host) + self._client = httpx.AsyncClient( + base_url=f"{pipeline_config.kafka_rest_host}/v3/clusters" + ) self._sync_client = httpx.Client(base_url=pipeline_config.kafka_rest_host) self._host = pipeline_config.kafka_rest_host @@ -64,7 +66,7 @@ async def create_topic(self, topic_spec: TopicSpec) -> None: :param topic_spec: The topic specification. """ response = await self._client.post( - url=f"/v3/clusters/{self.cluster_id}/topics", + url=f"/{self.cluster_id}/topics", headers=HEADERS, json=topic_spec.dict(exclude_none=True), ) @@ -83,7 +85,7 @@ async def delete_topic(self, topic_name: str) -> None: :param topic_name: Name of the topic """ response = await self._client.delete( - url=f"/v3/clusters/{self.cluster_id}/topics/{topic_name}", # type: ignore # FIXME https://github.com/ryananguiano/async_property/issues/17 + url=f"/{self.cluster_id}/topics/{topic_name}", headers=HEADERS, ) @@ -102,7 +104,7 @@ async def get_topic(self, topic_name: str) -> TopicResponse: """ response = await self._client.get( - url=f"/v3/clusters/{self.cluster_id}/topics/{topic_name}", # type: ignore # FIXME https://github.com/ryananguiano/async_property/issues/17 + url=f"/{self.cluster_id}/topics/{topic_name}", headers=HEADERS, ) @@ -130,7 +132,7 @@ async def get_topic_config(self, topic_name: str) -> TopicConfigResponse: """ response = await self._client.get( - url=f"/v3/clusters/{self.cluster_id}/topics/{topic_name}/configs", # type: ignore # FIXME https://github.com/ryananguiano/async_property/issues/17 + url=f"/{self.cluster_id}/topics/{topic_name}/configs", headers=HEADERS, ) @@ -159,7 +161,7 @@ async def batch_alter_topic_config( :param config_name: The configuration parameter name. """ response = await self._client.post( - url=f"/v3/clusters/{self.cluster_id}/topics/{topic_name}/configs:alter", # type: ignore # FIXME https://github.com/ryananguiano/async_property/issues/17 + url=f"/{self.cluster_id}/topics/{topic_name}/configs:alter", headers=HEADERS, json={"data": json_body}, ) @@ -177,7 +179,7 @@ async def get_broker_config(self) -> BrokerConfigResponse: :return: The broker configuration. """ response = await self._client.get( - url=f"/v3/clusters/{self.cluster_id}/brokers/-/configs", # type: ignore # FIXME https://github.com/ryananguiano/async_property/issues/17 + url=f"/{self.cluster_id}/brokers/-/configs", headers=HEADERS, ) diff --git a/tests/cli/snapshots/snap_test_schema_generation.py b/tests/cli/snapshots/snap_test_schema_generation.py index 5f9900de6..2eb0233fb 100644 --- a/tests/cli/snapshots/snap_test_schema_generation.py +++ b/tests/cli/snapshots/snap_test_schema_generation.py @@ -114,7 +114,7 @@ "type": "object" }, "InputTopicTypes": { - "description": "Input topic types\\n\\ninput (input topic), input_pattern (input pattern topic), extra (extra topic), extra_pattern (extra pattern topic).\\nEvery extra topic must have a role.", + "description": "Input topic types\\n\\n input (input topic), input_pattern (input pattern topic), extra (extra topic), extra_pattern (extra pattern topic).\\n Every extra topic must have a role.\\n ", "enum": [ "input", "extra", @@ -125,7 +125,7 @@ "type": "string" }, "OutputTopicTypes": { - "description": "Types of output topic\\n\\nError (error topic), output (output topic), and extra topics. Every extra topic must have a role.", + "description": "Types of output topic\\n\\n Error (error topic), output (output topic), and extra topics. Every extra topic must have a role.\\n ", "enum": [ "error", "output", diff --git a/tests/component_handlers/kafka_connect/test_connect_handler.py b/tests/component_handlers/kafka_connect/test_connect_handler.py index 9fba60a9a..11d0655f4 100644 --- a/tests/component_handlers/kafka_connect/test_connect_handler.py +++ b/tests/component_handlers/kafka_connect/test_connect_handler.py @@ -41,7 +41,7 @@ def log_error_mock(self, mocker: MockerFixture) -> MagicMock: ) @pytest.fixture - def connector_wrapper(self): + def connector_wrapper(self) -> AsyncMock: return AsyncMock() @pytest.fixture diff --git a/tests/component_handlers/kafka_connect/test_connect_wrapper.py b/tests/component_handlers/kafka_connect/test_connect_wrapper.py index 6aab50364..77a4d8d42 100644 --- a/tests/component_handlers/kafka_connect/test_connect_wrapper.py +++ b/tests/component_handlers/kafka_connect/test_connect_wrapper.py @@ -1,7 +1,7 @@ import json import sys from pathlib import Path -from unittest.mock import MagicMock, patch +from unittest.mock import AsyncMock, MagicMock, patch import pytest import pytest_asyncio @@ -51,7 +51,7 @@ def test_should_through_exception_when_host_is_not_set(self): @pytest.mark.asyncio @patch("httpx.AsyncClient.post") async def test_should_create_post_requests_for_given_connector_configuration( - self, mock_post: MagicMock + self, mock_post: AsyncMock ): configs = { "batch.size": "50", @@ -140,7 +140,7 @@ async def create_connector_locally(): @pytest.mark.asyncio @patch("httpx.AsyncClient.get") async def test_should_create_correct_get_connector_request( - self, mock_get: MagicMock + self, mock_get: AsyncMock ): connector_name = "test-connector" with pytest.raises(KafkaConnectError): @@ -258,7 +258,7 @@ async def get_connector_locally(): @pytest.mark.asyncio @patch("httpx.AsyncClient.put") async def test_should_create_correct_update_connector_request( - self, mock_put: MagicMock + self, mock_put: AsyncMock ): connector_name = "test-connector" configs = { @@ -392,7 +392,7 @@ async def update_connector_locally(): @pytest.mark.asyncio @patch("httpx.AsyncClient.delete") async def test_should_create_correct_delete_connector_request( - self, mock_delete: MagicMock + self, mock_delete: AsyncMock ): connector_name = "test-connector" with pytest.raises(KafkaConnectError): @@ -490,7 +490,7 @@ async def delete_connector_locally(): @pytest.mark.asyncio @patch("httpx.AsyncClient.put") async def test_should_create_correct_validate_connector_config_request( - self, mock_put: MagicMock + self, mock_put: AsyncMock ): configs = { "connector.class": "org.apache.kafka.connect.file.FileStreamSinkConnector", diff --git a/tests/component_handlers/schema_handler/test_schema_handler.py b/tests/component_handlers/schema_handler/test_schema_handler.py index f7487eca2..73fd6b7d3 100644 --- a/tests/component_handlers/schema_handler/test_schema_handler.py +++ b/tests/component_handlers/schema_handler/test_schema_handler.py @@ -47,7 +47,7 @@ def find_class_mock(mocker: MockerFixture) -> MagicMock: @pytest.fixture(autouse=True) -def schema_registry_mock(mocker: MockerFixture) -> MagicMock: +def schema_registry_mock(mocker: MockerFixture) -> AsyncMock: schema_registry_mock_constructor = mocker.patch( "kpops.component_handlers.schema_handler.schema_handler.AsyncSchemaRegistryClient", ) @@ -159,7 +159,7 @@ def test_should_raise_value_error_when_schema_provider_is_called_and_components_ @pytest.mark.asyncio async def test_should_log_info_when_submit_schemas_that_not_exists_and_dry_run_true( - to_section: ToSection, log_info_mock: MagicMock, schema_registry_mock: MagicMock + to_section: ToSection, log_info_mock: MagicMock, schema_registry_mock: AsyncMock ): schema_handler = SchemaHandler( url="http://mock:8081", components_module=TEST_SCHEMA_PROVIDER_MODULE diff --git a/tests/component_handlers/topic/test_proxy_wrapper.py b/tests/component_handlers/topic/test_proxy_wrapper.py index 90eb5421a..e8c81bbd8 100644 --- a/tests/component_handlers/topic/test_proxy_wrapper.py +++ b/tests/component_handlers/topic/test_proxy_wrapper.py @@ -49,7 +49,7 @@ async def setup(self, httpx_mock: HTTPXMock): status_code=200, ) assert self.proxy_wrapper.host == HOST - assert self.proxy_wrapper.cluster_id == "cluster-1" # type: ignore + assert self.proxy_wrapper.cluster_id == "cluster-1" @pytest.mark.asyncio async def test_should_raise_exception_when_host_is_not_set(self): @@ -81,7 +81,7 @@ async def test_should_create_topic_with_all_topic_configuration( await self.proxy_wrapper.create_topic(topic_spec=TopicSpec(**topic_spec)) mock_post.assert_called_with( - url=f"/v3/clusters/{self.proxy_wrapper.cluster_id}/topics", + url=f"/{self.proxy_wrapper.cluster_id}/topics", headers=HEADERS, json=topic_spec, ) @@ -97,27 +97,27 @@ async def test_should_create_topic_with_no_configuration( await self.proxy_wrapper.create_topic(topic_spec=TopicSpec(**topic_spec)) mock_post.assert_called_with( - url=f"/v3/clusters/{self.proxy_wrapper.cluster_id}/topics", + url=f"/{self.proxy_wrapper.cluster_id}/topics", headers=HEADERS, json=topic_spec, ) @pytest.mark.asyncio @patch("httpx.AsyncClient.get") - async def test_should_call_get_topic(self, mock_get: MagicMock): + async def test_should_call_get_topic(self, mock_get: AsyncMock): topic_name = "topic-X" with pytest.raises(KafkaRestProxyError): await self.proxy_wrapper.get_topic(topic_name=topic_name) mock_get.assert_called_with( - url=f"/v3/clusters/{self.proxy_wrapper.cluster_id}/topics/{topic_name}", + url=f"/{self.proxy_wrapper.cluster_id}/topics/{topic_name}", headers=HEADERS, ) @pytest.mark.asyncio @patch("httpx.AsyncClient.post") - async def test_should_call_batch_alter_topic_config(self, mock_put: MagicMock): + async def test_should_call_batch_alter_topic_config(self, mock_put: AsyncMock): topic_name = "topic-X" with pytest.raises(KafkaRestProxyError): @@ -130,7 +130,7 @@ async def test_should_call_batch_alter_topic_config(self, mock_put: MagicMock): ) mock_put.assert_called_with( - url=f"/v3/clusters/cluster-1/topics/{topic_name}/configs:alter", + url=f"/cluster-1/topics/{topic_name}/configs:alter", headers=HEADERS, json={ "data": [ @@ -142,25 +142,25 @@ async def test_should_call_batch_alter_topic_config(self, mock_put: MagicMock): @pytest.mark.asyncio @patch("httpx.AsyncClient.delete") - async def test_should_call_delete_topic(self, mock_delete: MagicMock): + async def test_should_call_delete_topic(self, mock_delete: AsyncMock): topic_name = "topic-X" with pytest.raises(KafkaRestProxyError): await self.proxy_wrapper.delete_topic(topic_name=topic_name) mock_delete.assert_called_with( - url=f"/v3/clusters/{self.proxy_wrapper.cluster_id}/topics/{topic_name}", + url=f"/{self.proxy_wrapper.cluster_id}/topics/{topic_name}", headers=HEADERS, ) @pytest.mark.asyncio @patch("httpx.AsyncClient.get") - async def test_should_call_get_broker_config(self, mock_get: MagicMock): + async def test_should_call_get_broker_config(self, mock_get: AsyncMock): with pytest.raises(KafkaRestProxyError): await self.proxy_wrapper.get_broker_config() mock_get.assert_called_with( - url=f"/v3/clusters/{self.proxy_wrapper.cluster_id}/brokers/-/configs", + url=f"/{self.proxy_wrapper.cluster_id}/brokers/-/configs", headers=HEADERS, ) From 10b5922591db71ff8329bf45348ef26cd6338e6d Mon Sep 17 00:00:00 2001 From: Alejandro Jaramillo Date: Tue, 8 Aug 2023 11:21:06 +0200 Subject: [PATCH 18/23] Implement changes --- tests/component_handlers/kafka_connect/test_connect_wrapper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/component_handlers/kafka_connect/test_connect_wrapper.py b/tests/component_handlers/kafka_connect/test_connect_wrapper.py index 77a4d8d42..bb0d9b675 100644 --- a/tests/component_handlers/kafka_connect/test_connect_wrapper.py +++ b/tests/component_handlers/kafka_connect/test_connect_wrapper.py @@ -513,7 +513,7 @@ async def test_should_create_correct_validate_connector_config_request( @pytest.mark.asyncio @patch("httpx.AsyncClient.put") async def test_should_create_correct_validate_connector_config_and_name_gets_added( - self, mock_put: MagicMock + self, mock_put: AsyncMock ): connector_name = "FileStreamSinkConnector" configs = { From 293d5ca19c26ace360fb092da5e305d974e42c85 Mon Sep 17 00:00:00 2001 From: Alejandro Jaramillo Date: Tue, 8 Aug 2023 11:23:03 +0200 Subject: [PATCH 19/23] Rollback snapshot --- tests/cli/snapshots/snap_test_schema_generation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/cli/snapshots/snap_test_schema_generation.py b/tests/cli/snapshots/snap_test_schema_generation.py index 2eb0233fb..5f9900de6 100644 --- a/tests/cli/snapshots/snap_test_schema_generation.py +++ b/tests/cli/snapshots/snap_test_schema_generation.py @@ -114,7 +114,7 @@ "type": "object" }, "InputTopicTypes": { - "description": "Input topic types\\n\\n input (input topic), input_pattern (input pattern topic), extra (extra topic), extra_pattern (extra pattern topic).\\n Every extra topic must have a role.\\n ", + "description": "Input topic types\\n\\ninput (input topic), input_pattern (input pattern topic), extra (extra topic), extra_pattern (extra pattern topic).\\nEvery extra topic must have a role.", "enum": [ "input", "extra", @@ -125,7 +125,7 @@ "type": "string" }, "OutputTopicTypes": { - "description": "Types of output topic\\n\\n Error (error topic), output (output topic), and extra topics. Every extra topic must have a role.\\n ", + "description": "Types of output topic\\n\\nError (error topic), output (output topic), and extra topics. Every extra topic must have a role.", "enum": [ "error", "output", From cf73c82c2582a223df83d15feb98a61d5abad21b Mon Sep 17 00:00:00 2001 From: Alejandro Jaramillo Date: Tue, 8 Aug 2023 11:47:13 +0200 Subject: [PATCH 20/23] Fix tests --- tests/cli/snapshots/snap_test_schema_generation.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/cli/snapshots/snap_test_schema_generation.py b/tests/cli/snapshots/snap_test_schema_generation.py index 5f9900de6..2eb0233fb 100644 --- a/tests/cli/snapshots/snap_test_schema_generation.py +++ b/tests/cli/snapshots/snap_test_schema_generation.py @@ -114,7 +114,7 @@ "type": "object" }, "InputTopicTypes": { - "description": "Input topic types\\n\\ninput (input topic), input_pattern (input pattern topic), extra (extra topic), extra_pattern (extra pattern topic).\\nEvery extra topic must have a role.", + "description": "Input topic types\\n\\n input (input topic), input_pattern (input pattern topic), extra (extra topic), extra_pattern (extra pattern topic).\\n Every extra topic must have a role.\\n ", "enum": [ "input", "extra", @@ -125,7 +125,7 @@ "type": "string" }, "OutputTopicTypes": { - "description": "Types of output topic\\n\\nError (error topic), output (output topic), and extra topics. Every extra topic must have a role.", + "description": "Types of output topic\\n\\n Error (error topic), output (output topic), and extra topics. Every extra topic must have a role.\\n ", "enum": [ "error", "output", From 097dae4e26a86d97a7b010ed1d44036ccbbf9b8a Mon Sep 17 00:00:00 2001 From: Alejandro Jaramillo Date: Tue, 8 Aug 2023 11:50:06 +0200 Subject: [PATCH 21/23] Fix test --- docs/docs/schema/pipeline.json | 4 ++-- tests/cli/snapshots/snap_test_schema_generation.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/docs/schema/pipeline.json b/docs/docs/schema/pipeline.json index 3f71896a1..76da0f23c 100644 --- a/docs/docs/schema/pipeline.json +++ b/docs/docs/schema/pipeline.json @@ -87,7 +87,7 @@ "type": "object" }, "InputTopicTypes": { - "description": "Input topic types\n\n input (input topic), input_pattern (input pattern topic), extra (extra topic), extra_pattern (extra pattern topic).\n Every extra topic must have a role.\n ", + "description": "Input topic types\n\ninput (input topic), input_pattern (input pattern topic), extra (extra topic), extra_pattern (extra pattern topic).\nEvery extra topic must have a role.", "enum": [ "input", "extra", @@ -641,7 +641,7 @@ "type": "object" }, "OutputTopicTypes": { - "description": "Types of output topic\n\n Error (error topic), output (output topic), and extra topics. Every extra topic must have a role.\n ", + "description": "Types of output topic\n\nError (error topic), output (output topic), and extra topics. Every extra topic must have a role.", "enum": [ "error", "output", diff --git a/tests/cli/snapshots/snap_test_schema_generation.py b/tests/cli/snapshots/snap_test_schema_generation.py index 2eb0233fb..5f9900de6 100644 --- a/tests/cli/snapshots/snap_test_schema_generation.py +++ b/tests/cli/snapshots/snap_test_schema_generation.py @@ -114,7 +114,7 @@ "type": "object" }, "InputTopicTypes": { - "description": "Input topic types\\n\\n input (input topic), input_pattern (input pattern topic), extra (extra topic), extra_pattern (extra pattern topic).\\n Every extra topic must have a role.\\n ", + "description": "Input topic types\\n\\ninput (input topic), input_pattern (input pattern topic), extra (extra topic), extra_pattern (extra pattern topic).\\nEvery extra topic must have a role.", "enum": [ "input", "extra", @@ -125,7 +125,7 @@ "type": "string" }, "OutputTopicTypes": { - "description": "Types of output topic\\n\\n Error (error topic), output (output topic), and extra topics. Every extra topic must have a role.\\n ", + "description": "Types of output topic\\n\\nError (error topic), output (output topic), and extra topics. Every extra topic must have a role.", "enum": [ "error", "output", From 6f7975887c90953d5c6b06fdc5607a2e2b00d71e Mon Sep 17 00:00:00 2001 From: Alejandro Jaramillo Date: Tue, 8 Aug 2023 14:29:26 +0200 Subject: [PATCH 22/23] Replace baseurl for sync client --- kpops/component_handlers/topic/proxy_wrapper.py | 4 ++-- tests/component_handlers/topic/test_proxy_wrapper.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/kpops/component_handlers/topic/proxy_wrapper.py b/kpops/component_handlers/topic/proxy_wrapper.py index aa96e7ff1..e8ec20d73 100644 --- a/kpops/component_handlers/topic/proxy_wrapper.py +++ b/kpops/component_handlers/topic/proxy_wrapper.py @@ -33,7 +33,7 @@ def __init__(self, pipeline_config: PipelineConfig) -> None: self._client = httpx.AsyncClient( base_url=f"{pipeline_config.kafka_rest_host}/v3/clusters" ) - self._sync_client = httpx.Client(base_url=pipeline_config.kafka_rest_host) + self._sync_client = httpx.Client(base_url=f"{pipeline_config.kafka_rest_host}/v3/clusters") self._host = pipeline_config.kafka_rest_host @cached_property @@ -47,7 +47,7 @@ def cluster_id(self) -> str: bootstrap.servers configuration. Therefore, only one Kafka cluster will be returned. :return: The Kafka cluster ID. """ - response = self._sync_client.get(url="/v3/clusters") + response = self._sync_client.get("") if response.status_code == httpx.codes.OK: cluster_information = response.json() diff --git a/tests/component_handlers/topic/test_proxy_wrapper.py b/tests/component_handlers/topic/test_proxy_wrapper.py index e8c81bbd8..146b427ca 100644 --- a/tests/component_handlers/topic/test_proxy_wrapper.py +++ b/tests/component_handlers/topic/test_proxy_wrapper.py @@ -44,7 +44,7 @@ async def setup(self, httpx_mock: HTTPXMock): httpx_mock.add_response( method="GET", - url=f"{HOST}/v3/clusters", + url=f"{HOST}/v3/clusters/", json=cluster_response, status_code=200, ) From 403ae42d17da7e4f84ec7f0a9ccf5c36586a0311 Mon Sep 17 00:00:00 2001 From: Alejandro Jaramillo Date: Tue, 8 Aug 2023 15:54:16 +0200 Subject: [PATCH 23/23] Reformat --- kpops/component_handlers/topic/proxy_wrapper.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/kpops/component_handlers/topic/proxy_wrapper.py b/kpops/component_handlers/topic/proxy_wrapper.py index e8ec20d73..8a3041574 100644 --- a/kpops/component_handlers/topic/proxy_wrapper.py +++ b/kpops/component_handlers/topic/proxy_wrapper.py @@ -33,7 +33,9 @@ def __init__(self, pipeline_config: PipelineConfig) -> None: self._client = httpx.AsyncClient( base_url=f"{pipeline_config.kafka_rest_host}/v3/clusters" ) - self._sync_client = httpx.Client(base_url=f"{pipeline_config.kafka_rest_host}/v3/clusters") + self._sync_client = httpx.Client( + base_url=f"{pipeline_config.kafka_rest_host}/v3/clusters" + ) self._host = pipeline_config.kafka_rest_host @cached_property