From b40cbd832185366493e83980d823eb034a189359 Mon Sep 17 00:00:00 2001 From: rashidakanchwala Date: Tue, 29 Oct 2024 17:00:06 +0000 Subject: [PATCH 1/7] done, test left Signed-off-by: rashidakanchwala --- package/kedro_viz/data_access/repositories/graph.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/package/kedro_viz/data_access/repositories/graph.py b/package/kedro_viz/data_access/repositories/graph.py index 601e52d06..c9a1f73f2 100644 --- a/package/kedro_viz/data_access/repositories/graph.py +++ b/package/kedro_viz/data_access/repositories/graph.py @@ -15,9 +15,13 @@ def has_node(self, node: GraphNode) -> bool: return node.id in self.nodes_dict def add_node(self, node: GraphNode) -> GraphNode: - if not self.has_node(node): + existing_node = self.nodes_dict.get(node.id) + if existing_node: + # Update tags or other attributes if the node already exists + existing_node.tags.update(node.tags) + else: self.nodes_dict[node.id] = node - self.nodes_list.append(node) + self.nodes_list.append(node) return self.nodes_dict[node.id] def get_node_by_id(self, node_id: str) -> Optional[GraphNode]: From b5e35cd0304547b2669106d239e911b6fdd4ce4b Mon Sep 17 00:00:00 2001 From: rashidakanchwala Date: Wed, 30 Oct 2024 09:38:12 +0000 Subject: [PATCH 2/7] lint Signed-off-by: rashidakanchwala --- package/kedro_viz/data_access/repositories/graph.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package/kedro_viz/data_access/repositories/graph.py b/package/kedro_viz/data_access/repositories/graph.py index c9a1f73f2..1254c88b7 100644 --- a/package/kedro_viz/data_access/repositories/graph.py +++ b/package/kedro_viz/data_access/repositories/graph.py @@ -21,7 +21,7 @@ def add_node(self, node: GraphNode) -> GraphNode: existing_node.tags.update(node.tags) else: self.nodes_dict[node.id] = node - self.nodes_list.append(node) + self.nodes_list.append(node) return self.nodes_dict[node.id] def get_node_by_id(self, node_id: str) -> Optional[GraphNode]: From e861b309f61219f7f914a9e8c1ac01c1afc49088 Mon Sep 17 00:00:00 2001 From: rashidakanchwala Date: Wed, 30 Oct 2024 10:02:27 +0000 Subject: [PATCH 3/7] tests done Signed-off-by: rashidakanchwala --- package/tests/conftest.py | 53 +++++++++++++++++++ .../test_api/test_rest/test_responses.py | 13 +++++ 2 files changed, 66 insertions(+) diff --git a/package/tests/conftest.py b/package/tests/conftest.py index 7c6605132..4c50749b9 100644 --- a/package/tests/conftest.py +++ b/package/tests/conftest.py @@ -380,6 +380,29 @@ def edge_case_example_pipelines( } +@pytest.fixture +def edge_case_example_pipelines_with_tags( + example_pipeline_with_dataset_as_input_and_output, + example_pipeline_with_dataset_as_input_to_outer_namespace, +): + """ + Fixture to mock the use cases mentioned in + https://github.com/kedro-org/kedro-viz/issues/2106 + """ + + pipelines_dict = { + "customer_pipeline": example_pipeline_with_dataset_as_input_and_output, + "car_pipeline": example_pipeline_with_dataset_as_input_to_outer_namespace, + } + + pipelines_dict["__default__"] = pipeline( + sum(pipeline for pipeline in pipelines_dict.values()), + tags=["default_tag1", "default_tag2"], + ) + + yield pipelines_dict + + @pytest.fixture def expected_modular_pipeline_tree_for_edge_cases(): expected_tree_for_edge_cases_file_path = ( @@ -538,6 +561,36 @@ def example_api_for_edge_case_pipelines( yield api +@pytest.fixture +def example_api_for_edge_case_pipelines_with_tags( + data_access_manager: DataAccessManager, + edge_case_example_pipelines_with_tags: Dict[str, Pipeline], + example_catalog: DataCatalog, + session_store: BaseSessionStore, + mocker, +): + api = apps.create_api_app_from_project(mock.MagicMock()) + + # For readability we are not hashing the node id + mocker.patch("kedro_viz.utils._hash", side_effect=lambda value: value) + mocker.patch( + "kedro_viz.data_access.repositories.modular_pipelines._hash", + side_effect=lambda value: value, + ) + + populate_data( + data_access_manager, + example_catalog, + edge_case_example_pipelines_with_tags, + session_store, + {}, + ) + mocker.patch( + "kedro_viz.api.rest.responses.data_access_manager", new=data_access_manager + ) + yield api + + @pytest.fixture def example_transcoded_api( data_access_manager: DataAccessManager, diff --git a/package/tests/test_api/test_rest/test_responses.py b/package/tests/test_api/test_rest/test_responses.py index 6f4581d3a..10df37799 100644 --- a/package/tests/test_api/test_rest/test_responses.py +++ b/package/tests/test_api/test_rest/test_responses.py @@ -594,6 +594,19 @@ def test_endpoint_main_for_edge_case_pipelines( actual_modular_pipelines_tree, expected_modular_pipeline_tree_for_edge_cases ) + def test_endpoint_main_for_edge_case_pipelines_with_tags( + self, + example_api_for_edge_case_pipelines_with_tags, + ): + expected_tags = [ + {"id": "default_tag1", "name": "default_tag1"}, + {"id": "default_tag2", "name": "default_tag2"}, + ] + client = TestClient(example_api_for_edge_case_pipelines_with_tags) + response = client.get("/api/main") + actual_tags = response.json()["tags"] + assert actual_tags == expected_tags + class TestTranscodedDataset: """Test a viz API created from a Kedro project.""" From a6791ed0f4a6a0cde0ad0025ec3e552071ddf7ba Mon Sep 17 00:00:00 2001 From: rashidakanchwala Date: Wed, 30 Oct 2024 10:21:33 +0000 Subject: [PATCH 4/7] fix test cov Signed-off-by: rashidakanchwala --- package/kedro_viz/data_access/repositories/graph.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/package/kedro_viz/data_access/repositories/graph.py b/package/kedro_viz/data_access/repositories/graph.py index 1254c88b7..3afb46ccd 100644 --- a/package/kedro_viz/data_access/repositories/graph.py +++ b/package/kedro_viz/data_access/repositories/graph.py @@ -11,9 +11,6 @@ def __init__(self): self.nodes_dict: Dict[str, GraphNode] = {} self.nodes_list: List[GraphNode] = [] - def has_node(self, node: GraphNode) -> bool: - return node.id in self.nodes_dict - def add_node(self, node: GraphNode) -> GraphNode: existing_node = self.nodes_dict.get(node.id) if existing_node: From 8b019270213d6593f1ebca456b10378178d4c15e Mon Sep 17 00:00:00 2001 From: rashidakanchwala Date: Thu, 31 Oct 2024 10:24:23 +0000 Subject: [PATCH 5/7] make simple tests Signed-off-by: rashidakanchwala --- package/tests/conftest.py | 23 ++++++++----------- .../test_api/test_rest/test_responses.py | 12 ++++++---- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/package/tests/conftest.py b/package/tests/conftest.py index 4c50749b9..e10ece696 100644 --- a/package/tests/conftest.py +++ b/package/tests/conftest.py @@ -221,6 +221,7 @@ def example_pipeline_with_node_namespaces(): inputs=["raw_transaction_data", "cleaned_transaction_data"], outputs="validated_transaction_data", name="validation_node", + tags=["validation"], ), node( func=lambda validated_data, enrichment_data: ( @@ -381,9 +382,8 @@ def edge_case_example_pipelines( @pytest.fixture -def edge_case_example_pipelines_with_tags( - example_pipeline_with_dataset_as_input_and_output, - example_pipeline_with_dataset_as_input_to_outer_namespace, +def example_pipelines_with_additional_tags( + example_pipeline_with_node_namespaces ): """ Fixture to mock the use cases mentioned in @@ -391,15 +391,10 @@ def edge_case_example_pipelines_with_tags( """ pipelines_dict = { - "customer_pipeline": example_pipeline_with_dataset_as_input_and_output, - "car_pipeline": example_pipeline_with_dataset_as_input_to_outer_namespace, + "pipeline": example_pipeline_with_node_namespaces, + "pipeline_with_tags": pipeline(example_pipeline_with_node_namespaces, tags=["tag1", "tag2"]), } - - pipelines_dict["__default__"] = pipeline( - sum(pipeline for pipeline in pipelines_dict.values()), - tags=["default_tag1", "default_tag2"], - ) - + yield pipelines_dict @@ -562,9 +557,9 @@ def example_api_for_edge_case_pipelines( @pytest.fixture -def example_api_for_edge_case_pipelines_with_tags( +def example_api_for_pipelines_with_additional_tags( data_access_manager: DataAccessManager, - edge_case_example_pipelines_with_tags: Dict[str, Pipeline], + example_pipelines_with_additional_tags: Dict[str, Pipeline], example_catalog: DataCatalog, session_store: BaseSessionStore, mocker, @@ -581,7 +576,7 @@ def example_api_for_edge_case_pipelines_with_tags( populate_data( data_access_manager, example_catalog, - edge_case_example_pipelines_with_tags, + example_pipelines_with_additional_tags, session_store, {}, ) diff --git a/package/tests/test_api/test_rest/test_responses.py b/package/tests/test_api/test_rest/test_responses.py index 10df37799..4523ad45a 100644 --- a/package/tests/test_api/test_rest/test_responses.py +++ b/package/tests/test_api/test_rest/test_responses.py @@ -594,15 +594,17 @@ def test_endpoint_main_for_edge_case_pipelines( actual_modular_pipelines_tree, expected_modular_pipeline_tree_for_edge_cases ) - def test_endpoint_main_for_edge_case_pipelines_with_tags( + def test_endpoint_main_for_pipelines_with_additional_tags( self, - example_api_for_edge_case_pipelines_with_tags, + example_api_for_pipelines_with_additional_tags, ): expected_tags = [ - {"id": "default_tag1", "name": "default_tag1"}, - {"id": "default_tag2", "name": "default_tag2"}, + {"id": "tag1", "name": "tag1"}, + {"id": "tag2", "name": "tag2"}, + {"id": "validation", "name": "validation"}, + ] - client = TestClient(example_api_for_edge_case_pipelines_with_tags) + client = TestClient(example_api_for_pipelines_with_additional_tags) response = client.get("/api/main") actual_tags = response.json()["tags"] assert actual_tags == expected_tags From 85bb7ed0eb83138fba0dd2612742e0dc601267bb Mon Sep 17 00:00:00 2001 From: rashidakanchwala Date: Thu, 31 Oct 2024 11:19:08 +0000 Subject: [PATCH 6/7] fixed lint Signed-off-by: rashidakanchwala --- package/tests/conftest.py | 10 +++++----- package/tests/test_api/test_rest/test_responses.py | 1 - 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/package/tests/conftest.py b/package/tests/conftest.py index e10ece696..be3225f47 100644 --- a/package/tests/conftest.py +++ b/package/tests/conftest.py @@ -382,9 +382,7 @@ def edge_case_example_pipelines( @pytest.fixture -def example_pipelines_with_additional_tags( - example_pipeline_with_node_namespaces -): +def example_pipelines_with_additional_tags(example_pipeline_with_node_namespaces): """ Fixture to mock the use cases mentioned in https://github.com/kedro-org/kedro-viz/issues/2106 @@ -392,9 +390,11 @@ def example_pipelines_with_additional_tags( pipelines_dict = { "pipeline": example_pipeline_with_node_namespaces, - "pipeline_with_tags": pipeline(example_pipeline_with_node_namespaces, tags=["tag1", "tag2"]), + "pipeline_with_tags": pipeline( + example_pipeline_with_node_namespaces, tags=["tag1", "tag2"] + ), } - + yield pipelines_dict diff --git a/package/tests/test_api/test_rest/test_responses.py b/package/tests/test_api/test_rest/test_responses.py index 4523ad45a..f7a78cd1a 100644 --- a/package/tests/test_api/test_rest/test_responses.py +++ b/package/tests/test_api/test_rest/test_responses.py @@ -602,7 +602,6 @@ def test_endpoint_main_for_pipelines_with_additional_tags( {"id": "tag1", "name": "tag1"}, {"id": "tag2", "name": "tag2"}, {"id": "validation", "name": "validation"}, - ] client = TestClient(example_api_for_pipelines_with_additional_tags) response = client.get("/api/main") From d975820405d73469d72ced1fee0b7d192e3133ac Mon Sep 17 00:00:00 2001 From: rashidakanchwala Date: Fri, 1 Nov 2024 10:52:20 +0000 Subject: [PATCH 7/7] fix lint and test Signed-off-by: rashidakanchwala --- package/tests/conftest.py | 7 ++++++- .../test_api/test_rest/test_responses/test_pipelines.py | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/package/tests/conftest.py b/package/tests/conftest.py index f2dcc9dce..ea25e94f7 100644 --- a/package/tests/conftest.py +++ b/package/tests/conftest.py @@ -597,7 +597,12 @@ def example_api_for_pipelines_with_additional_tags( {}, ) mocker.patch( - "kedro_viz.api.rest.responses.data_access_manager", new=data_access_manager + "kedro_viz.api.rest.responses.pipelines.data_access_manager", + new=data_access_manager, + ) + mocker.patch( + "kedro_viz.api.rest.responses.nodes.data_access_manager", + new=data_access_manager, ) yield api diff --git a/package/tests/test_api/test_rest/test_responses/test_pipelines.py b/package/tests/test_api/test_rest/test_responses/test_pipelines.py index 04b897d78..b1d14d8ca 100755 --- a/package/tests/test_api/test_rest/test_responses/test_pipelines.py +++ b/package/tests/test_api/test_rest/test_responses/test_pipelines.py @@ -34,7 +34,7 @@ def test_endpoint_main_no_default_pipeline(self, example_api_no_default_pipeline {"id": "data_science", "name": "data_science"}, {"id": "data_processing", "name": "data_processing"}, ] - + def test_endpoint_main_for_pipelines_with_additional_tags( self, example_api_for_pipelines_with_additional_tags,