Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE] ValidationConfig.save() #9579

Merged
merged 6 commits into from
Mar 7, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions great_expectations/core/validation_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,3 +222,13 @@ def run(
result_format=result_format,
)
return validator.validate_expectation_suite(self.suite, evaluation_parameters)

@public_api
def save(self) -> None:
store = project_manager.get_validation_config_store()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't we say we want the pattern here to be that we get the store upon initialization? (e.g. that's what we do for expectation suites). Both for raising earlier, and for predictability in case someone switches contexts. I think we could hijack a root_validator for this, even though I don't love it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah okay yeah wasn’t sure the pattern to follow when using Pydantic but you’re right

key = store.get_key(name=self.name, id=self.id)

try:
store.update(key=key, value=self)
except gx_exceptions.StoreBackendError:
raise ValueError("ValidationConfig must be added to a store before saving.")
11 changes: 11 additions & 0 deletions great_expectations/data_context/data_context/context_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@
ExpectationsStore,
ValidationsStore,
)
from great_expectations.data_context.store.validation_config_store import (
ValidationConfigStore,
)
from great_expectations.data_context.types.base import DataContextConfig
from great_expectations.datasource.datasource_dict import DatasourceDict
from great_expectations.datasource.fluent.batch_request import BatchRequest
Expand Down Expand Up @@ -105,6 +108,14 @@ def get_validations_store(self) -> ValidationsStore:
)
return self._project.validations_store

def get_validation_config_store(self) -> ValidationConfigStore:
if not self._project:
raise RuntimeError(
"This action requires an active DataContext. "
+ "Please call `great_expectations.get_context()` first, then try your action again."
)
return self._project.validation_config_store

def get_evaluation_parameters_store(self) -> EvaluationParameterStore:
if not self._project:
raise RuntimeError(
Expand Down
59 changes: 45 additions & 14 deletions tests/core/test_validation_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,21 +34,27 @@
)


class TestValidationRun:
@pytest.fixture
def validation_config(self) -> ValidationConfig:
context = gx.get_context(mode="ephemeral")
batch_config = (
context.sources.add_pandas("my_datasource")
.add_csv_asset("csv_asset", "taxi.csv") # type: ignore
.add_batch_config("my_batch_config")
)
return ValidationConfig(
name="my_validation",
data=batch_config,
suite=ExpectationSuite(name="my_suite"),
)
@pytest.fixture
def ephemeral_context():
return gx.get_context(mode="ephemeral")


@pytest.fixture
def validation_config(ephemeral_context: EphemeralDataContext) -> ValidationConfig:
context = ephemeral_context
batch_config = (
context.sources.add_pandas("my_datasource")
.add_csv_asset("csv_asset", "taxi.csv") # type: ignore
.add_batch_config("my_batch_config")
)
return ValidationConfig(
name="my_validation",
data=batch_config,
suite=ExpectationSuite(name="my_suite"),
)


class TestValidationRun:
@pytest.fixture
def mock_validator(self):
"""Set up our ProjectManager to return a mock Validator"""
Expand Down Expand Up @@ -491,3 +497,28 @@ def test_validation_config_deserialization_non_existant_resource(
):
with pytest.raises(ValueError, match=f"{error_substring}*."):
ValidationConfig.parse_obj(serialized_config)


@pytest.mark.unit
def test_validation_config_save_success(
ephemeral_context: EphemeralDataContext, validation_config: ValidationConfig
):
context = ephemeral_context
vc = validation_config

vc = context.validations.add(vc)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: to make the intention of the test here more clear, having the initial context.validations.add(vc) being done in a fixture might make sense, e.g. like a saved_validation_config fixture.


other_suite = ExpectationSuite(name="my_other_suite")
vc.suite = other_suite
vc.save()

assert vc.suite == other_suite
assert context.validations.get(vc.name).suite == other_suite


@pytest.mark.unit
def test_validation_config_save_failure(validation_config: ValidationConfig):
with pytest.raises(
ValueError, match="ValidationConfig must be added to a store before saving."
):
validation_config.save()
Loading