From f6e8ccc1c0d59b6b10bf6d7a088c4bf2e12f12a6 Mon Sep 17 00:00:00 2001 From: Chetan Kini Date: Mon, 19 Aug 2024 12:26:29 -0400 Subject: [PATCH] [MAINTENANCE] Raise informative errors if child objects are not persisted before parent (#10217) --- great_expectations/checkpoint/checkpoint.py | 31 ++- great_expectations/core/batch_definition.py | 9 + great_expectations/core/expectation_suite.py | 6 + .../core/validation_definition.py | 30 ++- great_expectations/exceptions/__init__.py | 1 - great_expectations/exceptions/exceptions.py | 53 +++- tests/checkpoint/test_checkpoint.py | 228 +++++++++++++++++- tests/core/test_batch_definition.py | 29 ++- tests/core/test_expectation_suite.py | 18 ++ tests/core/test_validation_definition.py | 116 ++++++++- tests/datasource/fluent/_fake_cloud_api.py | 13 +- 11 files changed, 505 insertions(+), 29 deletions(-) diff --git a/great_expectations/checkpoint/checkpoint.py b/great_expectations/checkpoint/checkpoint.py index 2c6dd2d33df7..aff2f4f03344 100644 --- a/great_expectations/checkpoint/checkpoint.py +++ b/great_expectations/checkpoint/checkpoint.py @@ -29,7 +29,12 @@ ExpectationSuiteIdentifier, ValidationResultIdentifier, ) -from great_expectations.exceptions.exceptions import CheckpointRunWithoutValidationDefinitionError +from great_expectations.exceptions.exceptions import ( + CheckpointNotAddedError, + CheckpointRelatedResourcesNotAddedError, + CheckpointRunWithoutValidationDefinitionError, + ResourceNotAddedError, +) from great_expectations.render.renderer.renderer import Renderer if TYPE_CHECKING: @@ -153,8 +158,13 @@ def run( if not self.validation_definitions: raise CheckpointRunWithoutValidationDefinitionError() - if not self.id: - self._add_to_store() + added, errors = self.is_added() + if not added: + # The checkpoint itself is not added but all children are - we can add it for the user + if len(errors) == 1 and isinstance(errors[0], CheckpointNotAddedError): + self._add_to_store() + else: + raise CheckpointRelatedResourcesNotAddedError(errors=errors) run_id = run_id or RunIdentifier(run_time=dt.datetime.now(dt.timezone.utc)) run_results = self._run_validation_definitions( @@ -252,6 +262,21 @@ def _sort_actions(self) -> List[CheckpointAction]: return priority_actions + secondary_actions + def is_added(self) -> tuple[bool, list[ResourceNotAddedError]]: + errs: list[ResourceNotAddedError] = [] + + validations_added: bool = True + for validation_definition in self.validation_definitions: + validation_added, validation_errs = validation_definition.is_added() + errs.extend(validation_errs) + validations_added = validation_added and validations_added + + self_added = self.id is not None + if not self_added: + errs.append(CheckpointNotAddedError(name=self.name)) + + return (validations_added and self_added, errs) + @public_api def save(self) -> None: from great_expectations.data_context.data_context.context_factory import project_manager diff --git a/great_expectations/core/batch_definition.py b/great_expectations/core/batch_definition.py index db084ec78bbb..3092c1ef532e 100644 --- a/great_expectations/core/batch_definition.py +++ b/great_expectations/core/batch_definition.py @@ -8,6 +8,10 @@ # Partitioner class when we update forward refs, so we just import here. from great_expectations.core.partitioners import ColumnPartitioner, FileNamePartitioner from great_expectations.core.serdes import _EncodedValidationData, _IdentifierBundle +from great_expectations.exceptions.exceptions import ( + BatchDefinitionNotAddedError, + ResourceNotAddedError, +) if TYPE_CHECKING: from great_expectations.datasource.fluent.batch_request import ( @@ -72,6 +76,11 @@ def get_batch(self, batch_parameters: Optional[BatchParameters] = None) -> Batch return batch_list[-1] + def is_added(self) -> tuple[bool, list[ResourceNotAddedError]]: + if self.id: + return True, [] + return False, [BatchDefinitionNotAddedError(name=self.name)] + def identifier_bundle(self) -> _EncodedValidationData: # Utilized as a custom json_encoder asset = self.data_asset diff --git a/great_expectations/core/expectation_suite.py b/great_expectations/core/expectation_suite.py index 53e0db3aa40a..c943e5f439eb 100644 --- a/great_expectations/core/expectation_suite.py +++ b/great_expectations/core/expectation_suite.py @@ -32,6 +32,7 @@ from great_expectations.core.serdes import _IdentifierBundle from great_expectations.exceptions.exceptions import ( ExpectationSuiteNotAddedError, + ResourceNotAddedError, ) from great_expectations.types import SerializableDictDot from great_expectations.util import ( @@ -233,6 +234,11 @@ def save(self) -> None: key = self._store.get_key(name=self.name, id=self.id) self._store.update(key=key, value=self) + def is_added(self) -> tuple[bool, list[ResourceNotAddedError]]: + if self.id: + return True, [] + return False, [ExpectationSuiteNotAddedError(name=self.name)] + def _has_been_saved(self) -> bool: """Has this ExpectationSuite been persisted to a Store?""" # todo: this should only check local keys instead of potentially querying the remote backend diff --git a/great_expectations/core/validation_definition.py b/great_expectations/core/validation_definition.py index 3a43be0d8f85..8b5385bb2469 100644 --- a/great_expectations/core/validation_definition.py +++ b/great_expectations/core/validation_definition.py @@ -27,7 +27,11 @@ GXCloudIdentifier, ValidationResultIdentifier, ) -from great_expectations.exceptions.exceptions import ValidationDefinitionNotAddedError +from great_expectations.exceptions.exceptions import ( + ResourceNotAddedError, + ValidationDefinitionNotAddedError, + ValidationDefinitionRelatedResourcesNotAddedError, +) from great_expectations.validator.v1_validator import Validator if TYPE_CHECKING: @@ -114,6 +118,21 @@ def asset(self) -> DataAsset: def data_source(self) -> Datasource: return self.asset.datasource + def is_added(self) -> tuple[bool, list[ResourceNotAddedError]]: + errors: list[ResourceNotAddedError] = [] + + data_added, data_errors = self.data.is_added() + errors.extend(data_errors) + + suite_added, suite_errors = self.suite.is_added() + errors.extend(suite_errors) + + self_added = self.id is not None + if not self_added: + errors.append(ValidationDefinitionNotAddedError(name=self.name)) + + return (data_added and suite_added and self_added, errors) + @validator("suite", pre=True) def _validate_suite(cls, v: dict | ExpectationSuite): # Input will be a dict of identifiers if being deserialized or a suite object if being constructed by a user. # noqa: E501 @@ -201,8 +220,13 @@ def run( result_format: ResultFormat | dict = ResultFormat.SUMMARY, run_id: RunIdentifier | None = None, ) -> ExpectationSuiteValidationResult: - if not self.id: - self._add_to_store() + added, errors = self.is_added() + if not added: + # The validation definition itself is not added but all children are - we can add it for the user # noqa: E501 + if len(errors) == 1 and isinstance(errors[0], ValidationDefinitionNotAddedError): + self._add_to_store() + else: + raise ValidationDefinitionRelatedResourcesNotAddedError(errors=errors) validator = Validator( batch_definition=self.batch_definition, diff --git a/great_expectations/exceptions/__init__.py b/great_expectations/exceptions/__init__.py index 6a39995a1176..aad700281f90 100644 --- a/great_expectations/exceptions/__init__.py +++ b/great_expectations/exceptions/__init__.py @@ -71,7 +71,6 @@ StoreBackendUnsupportedResourceTypeError, StoreConfigurationError, StoreError, - SuiteEditNotebookCustomTemplateModuleNotFoundError, SuiteParameterError, UnavailableMetricError, UnsupportedConfigVersionError, diff --git a/great_expectations/exceptions/exceptions.py b/great_expectations/exceptions/exceptions.py index 9c3703c04399..fcd798ed5f02 100644 --- a/great_expectations/exceptions/exceptions.py +++ b/great_expectations/exceptions/exceptions.py @@ -34,29 +34,33 @@ def __str__(self) -> str: return self.message -class SuiteEditNotebookCustomTemplateModuleNotFoundError(ModuleNotFoundError): - def __init__(self, custom_module) -> None: - message = f"The custom module '{custom_module}' could not be found" - super().__init__(message) - - class DataContextError(GreatExpectationsError): pass -class ExpectationSuiteError(DataContextError): +class ResourceNotAddedError(DataContextError): pass -class ExpectationSuiteNotSavedError(DataContextError): +class ResourcesNotAddedError(ValueError): + def __init__(self, errors: list[ResourceNotAddedError]) -> None: + self._errors = errors + super().__init__("\n\t" + "\n\t".join(str(e) for e in errors)) + + @property + def errors(self) -> list[ResourceNotAddedError]: + return self._errors + + +class ExpectationSuiteError(DataContextError): pass -class ExpectationSuiteNotAddedError(ExpectationSuiteError): +class ExpectationSuiteNotAddedError(ResourceNotAddedError): def __init__(self, name: str) -> None: super().__init__( f"ExpectationSuite '{name}' must be added to the DataContext before it can be updated. " - "Please call context.suites.add(), " + "Please call `context.suites.add()`, " "then try your action again." ) @@ -65,11 +69,11 @@ class ValidationDefinitionError(DataContextError): pass -class ValidationDefinitionNotAddedError(ValidationDefinitionError): +class ValidationDefinitionNotAddedError(ResourceNotAddedError): def __init__(self, name: str) -> None: super().__init__( f"ValidationDefinition '{name}' must be added to the DataContext before it can be updated. " # noqa: E501 - "Please call context.validation_definitions.add(), " + "Please call `context.validation_definitions.add()`, " "then try your action again." ) @@ -82,6 +86,15 @@ class CheckpointNotFoundError(CheckpointError): pass +class CheckpointNotAddedError(ResourceNotAddedError): + def __init__(self, name: str) -> None: + super().__init__( + f"Checkpoint '{name}' must be added to the DataContext before it can be updated. " + "Please call `context.checkpoints.add()`, " + "then try your action again." + ) + + class CheckpointRunWithoutValidationDefinitionError(CheckpointError): def __init__(self) -> None: super().__init__( @@ -90,6 +103,14 @@ def __init__(self) -> None: ) +class CheckpointRelatedResourcesNotAddedError(ResourcesNotAddedError): + pass + + +class ValidationDefinitionRelatedResourcesNotAddedError(ResourcesNotAddedError): + pass + + class StoreBackendError(DataContextError): pass @@ -416,6 +437,14 @@ def __init__(self, message) -> None: super().__init__(self.message) +class BatchDefinitionNotAddedError(ResourceNotAddedError): + def __init__(self, name: str) -> None: + super().__init__( + f"BatchDefinition '{name}' must be added to the DataContext before it can be updated. " + "Please update using the parent asset or data source, then try your action again." + ) + + class BatchSpecError(DataContextError): def __init__(self, message) -> None: self.message = message diff --git a/tests/checkpoint/test_checkpoint.py b/tests/checkpoint/test_checkpoint.py index 76a5b66d0328..a5cb322a11f7 100644 --- a/tests/checkpoint/test_checkpoint.py +++ b/tests/checkpoint/test_checkpoint.py @@ -3,7 +3,7 @@ import json import pathlib import uuid -from typing import TYPE_CHECKING, List +from typing import TYPE_CHECKING, List, Type from unittest import mock import pytest @@ -41,7 +41,15 @@ from great_expectations.data_context.types.resource_identifiers import ( ValidationResultIdentifier, ) -from great_expectations.exceptions.exceptions import CheckpointRunWithoutValidationDefinitionError +from great_expectations.exceptions.exceptions import ( + BatchDefinitionNotAddedError, + CheckpointNotAddedError, + CheckpointRelatedResourcesNotAddedError, + CheckpointRunWithoutValidationDefinitionError, + ExpectationSuiteNotAddedError, + ResourceNotAddedError, + ValidationDefinitionNotAddedError, +) from great_expectations.expectations.expectation_configuration import ExpectationConfiguration from tests.test_utils import working_directory @@ -419,16 +427,20 @@ class TestCheckpointResult: def mock_suite(self, mocker: MockerFixture): suite = mocker.Mock(spec=ExpectationSuite) suite.name = self.suite_name + suite.is_added.return_value = (True, []) return suite @pytest.fixture def mock_batch_def(self, mocker: MockerFixture): - return mocker.Mock(spec=BatchDefinition) + bd = mocker.Mock(spec=BatchDefinition) + bd._copy_and_set_values().is_added.return_value = (True, []) + return bd @pytest.fixture def validation_definition(self, mock_suite: MockerFixture, mock_batch_def: MockerFixture): validation_definition = ValidationDefinition( name=self.validation_definition_name, + id=str(uuid.uuid4()), data=mock_batch_def, suite=mock_suite, ) @@ -483,6 +495,23 @@ def test_checkpoint_run_no_validation_definitions(self, mocker): with pytest.raises(CheckpointRunWithoutValidationDefinitionError): checkpoint.run() + @pytest.mark.unit + def test_checkpoint_run_dependencies_not_added_raises_error( + self, validation_definition: ValidationDefinition + ): + validation_definition.id = None + checkpoint = Checkpoint( + name=self.checkpoint_name, validation_definitions=[validation_definition] + ) + + with pytest.raises(CheckpointRelatedResourcesNotAddedError) as e: + checkpoint.run() + + assert [type(err) for err in e.value.errors] == [ + ValidationDefinitionNotAddedError, + CheckpointNotAddedError, + ] + @pytest.mark.unit def test_checkpoint_run_no_actions(self, validation_definition: ValidationDefinition): checkpoint = Checkpoint( @@ -779,3 +808,196 @@ def test_checkpoint_run_adds_requisite_ids(self, tmp_path: pathlib.Path): ] assert meta["checkpoint_id"] == checkpoint.id assert meta["validation_id"] == checkpoint.validation_definitions[0].id + + +@pytest.mark.parametrize( + "id,validation_def_id,suite_id,batch_def_id,is_added,error_list", + [ + pytest.param( + str(uuid.uuid4()), + str(uuid.uuid4()), + str(uuid.uuid4()), + str(uuid.uuid4()), + True, + [], + id="checkpoint_id|validation_id|suite_id|batch_def_id", + ), + pytest.param( + str(uuid.uuid4()), + str(uuid.uuid4()), + str(uuid.uuid4()), + None, + False, + [BatchDefinitionNotAddedError], + id="checkpoint_id|validation_id|suite_id|no_batch_def_id", + ), + pytest.param( + str(uuid.uuid4()), + str(uuid.uuid4()), + None, + str(uuid.uuid4()), + False, + [ExpectationSuiteNotAddedError], + id="checkpoint_id|validation_id|no_suite_id|batch_def_id", + ), + pytest.param( + str(uuid.uuid4()), + str(uuid.uuid4()), + None, + None, + False, + [BatchDefinitionNotAddedError, ExpectationSuiteNotAddedError], + id="checkpoint_id|validation_id|no_suite_id|no_batch_def_id", + ), + pytest.param( + str(uuid.uuid4()), + None, + str(uuid.uuid4()), + str(uuid.uuid4()), + False, + [ValidationDefinitionNotAddedError], + id="checkpoint_id|no_validation_id|suite_id|batch_def_id", + ), + pytest.param( + str(uuid.uuid4()), + None, + str(uuid.uuid4()), + None, + False, + [BatchDefinitionNotAddedError, ValidationDefinitionNotAddedError], + id="checkpoint_id|no_validation_id|suite_id|no_batch_def_id", + ), + pytest.param( + str(uuid.uuid4()), + None, + None, + str(uuid.uuid4()), + False, + [ExpectationSuiteNotAddedError, ValidationDefinitionNotAddedError], + id="checkpoint_id|no_validation_id|no_suite_id|batch_def_id", + ), + pytest.param( + str(uuid.uuid4()), + None, + None, + None, + False, + [ + BatchDefinitionNotAddedError, + ExpectationSuiteNotAddedError, + ValidationDefinitionNotAddedError, + ], + id="checkpoint_id|no_validation_id|no_suite_id|no_batch_def_id", + ), + pytest.param( + None, + str(uuid.uuid4()), + str(uuid.uuid4()), + str(uuid.uuid4()), + False, + [CheckpointNotAddedError], + id="no_checkpoint_id|validation_id|suite_id|batch_def_id", + ), + pytest.param( + None, + str(uuid.uuid4()), + str(uuid.uuid4()), + None, + False, + [BatchDefinitionNotAddedError, CheckpointNotAddedError], + id="no_checkpoint_id|validation_id|suite_id|no_batch_def_id", + ), + pytest.param( + None, + str(uuid.uuid4()), + None, + str(uuid.uuid4()), + False, + [ExpectationSuiteNotAddedError, CheckpointNotAddedError], + id="no_checkpoint_id|validation_id|no_suite_id|batch_def_id", + ), + pytest.param( + None, + str(uuid.uuid4()), + None, + None, + False, + [BatchDefinitionNotAddedError, ExpectationSuiteNotAddedError, CheckpointNotAddedError], + id="no_checkpoint_id|validation_id|no_suite_id|no_batch_def_id", + ), + pytest.param( + None, + None, + str(uuid.uuid4()), + str(uuid.uuid4()), + False, + [ValidationDefinitionNotAddedError, CheckpointNotAddedError], + id="no_checkpoint_id|no_validation_id|suite_id|batch_def_id", + ), + pytest.param( + None, + None, + str(uuid.uuid4()), + None, + False, + [ + BatchDefinitionNotAddedError, + ValidationDefinitionNotAddedError, + CheckpointNotAddedError, + ], + id="no_checkpoint_id|no_validation_id|suite_id|no_batch_def_id", + ), + pytest.param( + None, + None, + None, + str(uuid.uuid4()), + False, + [ + ExpectationSuiteNotAddedError, + ValidationDefinitionNotAddedError, + CheckpointNotAddedError, + ], + id="no_checkpoint_id|no_validation_id|no_suite_id|batch_def_id", + ), + pytest.param( + None, + None, + None, + None, + False, + [ + BatchDefinitionNotAddedError, + ExpectationSuiteNotAddedError, + ValidationDefinitionNotAddedError, + CheckpointNotAddedError, + ], + id="no_checkpoint_id|no_validation_id|no_suite_id|no_batch_def_id", + ), + ], +) +@pytest.mark.unit +def test_is_added( + id: str | None, + validation_def_id: str | None, + suite_id: str | None, + batch_def_id: str | None, + is_added: bool, + error_list: list[Type[ResourceNotAddedError]], +): + checkpoint = Checkpoint( + name="my_checkpoint", + id=id, + validation_definitions=[ + ValidationDefinition( + name="my_validation_definition", + id=validation_def_id, + suite=ExpectationSuite(name="my_suite", id=suite_id), + data=BatchDefinition(name="my_batch_def", id=batch_def_id), + ) + ], + ) + checkpoint_added, errors = checkpoint.is_added() + + assert checkpoint_added == is_added + assert [type(err) for err in errors] == error_list diff --git a/tests/core/test_batch_definition.py b/tests/core/test_batch_definition.py index 25d7c1340339..ce208d257eed 100644 --- a/tests/core/test_batch_definition.py +++ b/tests/core/test_batch_definition.py @@ -1,8 +1,8 @@ from __future__ import annotations import re +import uuid from typing import TYPE_CHECKING, Optional -from unittest.mock import Mock # noqa: TID251 import pytest @@ -12,14 +12,17 @@ from great_expectations.datasource.fluent.batch_request import BatchParameters from great_expectations.datasource.fluent.interfaces import Batch, DataAsset from great_expectations.datasource.fluent.pandas_datasource import PandasDatasource +from great_expectations.exceptions.exceptions import ( + BatchDefinitionNotAddedError, +) if TYPE_CHECKING: import pytest_mock @pytest.fixture -def mock_data_asset(monkeypatch) -> DataAsset: - monkeypatch.setattr(DataAsset, "build_batch_request", Mock()) +def mock_data_asset(monkeypatch, mocker: pytest_mock.MockerFixture) -> DataAsset: + monkeypatch.setattr(DataAsset, "build_batch_request", mocker.Mock()) data_asset: DataAsset = DataAsset(name="my_data_asset", type="table") return data_asset @@ -36,6 +39,7 @@ def mock_data_asset(monkeypatch) -> DataAsset: def test_build_batch_request( batch_parameters: Optional[BatchParameters], mock_data_asset: DataAsset, + mocker: pytest_mock.MockerFixture, ): batching_regex = re.compile(r"data_(?P\d{4})-(?P\d{2}).csv") partitioner = FileNamePartitionerYearly(regex=batching_regex) @@ -48,7 +52,7 @@ def test_build_batch_request( batch_definition.build_batch_request(batch_parameters=batch_parameters) mock_build_batch_request = batch_definition.data_asset.build_batch_request - assert isinstance(mock_build_batch_request, Mock) + assert isinstance(mock_build_batch_request, mocker.Mock) mock_build_batch_request.assert_called_once_with( options=batch_parameters, partitioner=partitioner, @@ -135,3 +139,20 @@ def test_identifier_bundle(): asset=_IdentifierBundle(name="my_asset", id=None), batch_definition=_IdentifierBundle(name="my_batch_definition", id=None), ) + + +@pytest.mark.parametrize( + "id,is_added,num_errors", + [ + pytest.param(str(uuid.uuid4()), True, 0, id="added"), + pytest.param(None, False, 1, id="not_added"), + ], +) +@pytest.mark.unit +def test_is_added(id: str | None, is_added: bool, num_errors: int): + batch_definition = BatchDefinition(name="my_batch_def", id=id) + batch_def_added, errors = batch_definition.is_added() + + assert batch_def_added == is_added + assert len(errors) == num_errors + assert all(isinstance(err, BatchDefinitionNotAddedError) for err in errors) diff --git a/tests/core/test_expectation_suite.py b/tests/core/test_expectation_suite.py index 994331dd8e0a..4dd48778cb34 100644 --- a/tests/core/test_expectation_suite.py +++ b/tests/core/test_expectation_suite.py @@ -1,5 +1,6 @@ from __future__ import annotations +import uuid from copy import copy, deepcopy from typing import Dict, Union from unittest import mock @@ -1063,3 +1064,20 @@ def test_identifier_bundle_no_id(): with pytest.raises(gx_exceptions.ExpectationSuiteNotAddedError): suite.identifier_bundle() + + +@pytest.mark.parametrize( + "id,is_added,num_errors", + [ + pytest.param(str(uuid.uuid4()), True, 0, id="added"), + pytest.param(None, False, 1, id="not_added"), + ], +) +@pytest.mark.unit +def test_is_added(id: str | None, is_added: bool, num_errors: int): + suite = ExpectationSuite(name="my_suite", id=id) + suite_added, errors = suite.is_added() + + assert suite_added == is_added + assert len(errors) == num_errors + assert all(isinstance(err, gx_exceptions.ExpectationSuiteNotAddedError) for err in errors) diff --git a/tests/core/test_validation_definition.py b/tests/core/test_validation_definition.py index b30dcf124e12..31df4a8ec2cf 100644 --- a/tests/core/test_validation_definition.py +++ b/tests/core/test_validation_definition.py @@ -2,7 +2,7 @@ import json import uuid -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Type from unittest import mock import pytest @@ -40,7 +40,13 @@ PandasDatasource, _PandasDataAsset, ) -from great_expectations.exceptions.exceptions import ValidationDefinitionNotAddedError +from great_expectations.exceptions.exceptions import ( + BatchDefinitionNotAddedError, + ExpectationSuiteNotAddedError, + ResourceNotAddedError, + ValidationDefinitionNotAddedError, + ValidationDefinitionRelatedResourcesNotAddedError, +) from great_expectations.execution_engine.execution_engine import ExecutionEngine from great_expectations.expectations.expectation_configuration import ( ExpectationConfiguration, @@ -321,6 +327,19 @@ def test_cloud_validation_def_creates_rendered_content( assert result.results[0].expectation_config.rendered_content is not None assert result.results[0].rendered_content is not None + @pytest.mark.unit + def test_dependencies_not_added_raises_error(self, validation_definition: ValidationDefinition): + validation_definition.suite.id = None + validation_definition.data.id = None + + with pytest.raises(ValidationDefinitionRelatedResourcesNotAddedError) as e: + validation_definition.run() + + assert [type(err) for err in e.value.errors] == [ + BatchDefinitionNotAddedError, + ExpectationSuiteNotAddedError, + ] + class TestValidationDefinitionSerialization: ds_name = "my_ds" @@ -669,3 +688,96 @@ def test_save_success(mocker: MockerFixture, validation_definition: ValidationDe context.validation_definition_store.update.assert_called_once_with( key=store_key, value=validation_definition ) + + +@pytest.mark.parametrize( + "id,suite_id,batch_def_id,is_added,error_list", + [ + pytest.param( + str(uuid.uuid4()), + str(uuid.uuid4()), + str(uuid.uuid4()), + True, + [], + id="validation_id|suite_id|batch_def_id", + ), + pytest.param( + str(uuid.uuid4()), + None, + str(uuid.uuid4()), + False, + [ExpectationSuiteNotAddedError], + id="validation_id|no_suite_id|batch_def_id", + ), + pytest.param( + str(uuid.uuid4()), + str(uuid.uuid4()), + None, + False, + [BatchDefinitionNotAddedError], + id="validation_id|suite_id|no_batch_def_id", + ), + pytest.param( + str(uuid.uuid4()), + None, + None, + False, + [BatchDefinitionNotAddedError, ExpectationSuiteNotAddedError], + id="validation_id|no_suite_id|no_batch_def_id", + ), + pytest.param( + None, + str(uuid.uuid4()), + str(uuid.uuid4()), + False, + [ValidationDefinitionNotAddedError], + id="no_validation_id|suite_id|batch_def_id", + ), + pytest.param( + None, + None, + str(uuid.uuid4()), + False, + [ExpectationSuiteNotAddedError, ValidationDefinitionNotAddedError], + id="no_validation_id|no_suite_id|batch_def_id", + ), + pytest.param( + None, + str(uuid.uuid4()), + None, + False, + [BatchDefinitionNotAddedError, ValidationDefinitionNotAddedError], + id="no_validation_id|suite_id|no_batch_def_id", + ), + pytest.param( + None, + None, + None, + False, + [ + BatchDefinitionNotAddedError, + ExpectationSuiteNotAddedError, + ValidationDefinitionNotAddedError, + ], + id="no_validation_id|no_suite_id|no_batch_def_id", + ), + ], +) +@pytest.mark.unit +def test_is_added( + id: str | None, + suite_id: str | None, + batch_def_id: str | None, + is_added: bool, + error_list: list[Type[ResourceNotAddedError]], +): + validation_definition = ValidationDefinition( + name="my_validation_definition", + id=id, + suite=ExpectationSuite(name="my_suite", id=suite_id), + data=BatchDefinition(name="my_batch_def", id=batch_def_id), + ) + validation_definition_added, errors = validation_definition.is_added() + + assert validation_definition_added == is_added + assert [type(err) for err in errors] == error_list diff --git a/tests/datasource/fluent/_fake_cloud_api.py b/tests/datasource/fluent/_fake_cloud_api.py index aef0f487db17..c03c07556f1f 100644 --- a/tests/datasource/fluent/_fake_cloud_api.py +++ b/tests/datasource/fluent/_fake_cloud_api.py @@ -12,6 +12,7 @@ Dict, Final, Generator, + List, NamedTuple, Optional, Sequence, @@ -64,6 +65,7 @@ class _DatasourceSchema(pydantic.BaseModel, extra="allow"): id: Optional[str] = None type: str name: str + assets: List[dict] = pydantic.Field(default_factory=list) class CloudResponseSchema(pydantic.BaseModel): @@ -384,7 +386,7 @@ def post_datasources_cb( ) -def put_datasource_cb(request: PreparedRequest) -> CallbackResult: +def put_datasource_cb(request: PreparedRequest) -> CallbackResult: # noqa: C901 LOGGER.debug(f"{request.method} {request.url}") if not request.url: raise NotImplementedError("request.url should not be empty") @@ -401,6 +403,15 @@ def put_datasource_cb(request: PreparedRequest) -> CallbackResult: parsed_url = urllib.parse.urlparse(request.url) datasource_id = parsed_url.path.split("/")[-1] + # Ensure that assets and batch definitions get IDs + # Note that this logic should also happen in POST but is not implemented for our fake + for asset in payload.data.assets: + if not asset.get("id"): + asset["id"] = str(uuid.uuid4()) + for batch_definition in asset.get("batch_definitions", []): + if not batch_definition.get("id"): + batch_definition["id"] = str(uuid.uuid4()) + old_datasource: dict | None = _CLOUD_API_FAKE_DB["datasources"].get(datasource_id) if old_datasource: if payload.data.name != old_datasource["data"]["name"]: