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] Checkpoint.save() #9676

Merged
merged 16 commits into from
Apr 1, 2024
7 changes: 6 additions & 1 deletion great_expectations/checkpoint/v1_checkpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,12 @@ def _run_actions(

@public_api
def save(self) -> None:
raise NotImplementedError
from great_expectations import project_manager

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

store.update(key=key, value=self)


class CheckpointResult(BaseModel):
Expand Down
6 changes: 1 addition & 5 deletions great_expectations/core/factory/checkpoint_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@

if TYPE_CHECKING:
from great_expectations.core.data_context_key import StringKey
from great_expectations.data_context.data_context.abstract_data_context import (
AbstractDataContext,
)
from great_expectations.data_context.store.checkpoint_store import (
V1CheckpointStore as CheckpointStore,
)
Expand All @@ -21,9 +18,8 @@

# TODO: Add analytics as needed
class CheckpointFactory(Factory[Checkpoint]):
def __init__(self, store: CheckpointStore, context: AbstractDataContext):
Copy link
Member

Choose a reason for hiding this comment

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

🥳

def __init__(self, store: CheckpointStore):
self._store = store
self._context = context

@public_api
@override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -325,16 +325,11 @@ def _init_factories(self) -> None:
)

self._checkpoints: CheckpointFactory | None = None
if checkpoint_store := self.stores.get(self.checkpoint_store_name):
if self.stores.get(self.checkpoint_store_name):
# NOTE: Currently in an intermediate state where both the legacy and V1 stores exist.
# Upon the deletion of the old checkpoint store, the new one will be promoted.
v1_checkpoint_store = V1CheckpointStore()
v1_checkpoint_store._store_backend = (
checkpoint_store.store_backend
) # Leverage same backend as what was configured for the old store
self._checkpoints = CheckpointFactory(
store=v1_checkpoint_store,
context=self,
store=self.v1_checkpoint_store,
)

self._validation_definitions: ValidationDefinitionFactory = ValidationDefinitionFactory(
Expand Down Expand Up @@ -654,6 +649,18 @@ def checkpoint_store_name(self, value: str) -> None:
def checkpoint_store(self) -> CheckpointStore:
return self.stores[self.checkpoint_store_name]

@property
def v1_checkpoint_store(self) -> V1CheckpointStore:
# Temporary property until the legacy checkpoint store is removed
# and the new checkpoint store is promoted to the old namespace
legacy_checkpoint_store = self.checkpoint_store
store = V1CheckpointStore()

# Leverage same backend as what was configured for the old store
store._store_backend = legacy_checkpoint_store.store_backend

return store

@property
def assistants(self) -> DataAssistantDispatcher:
return self._assistants
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@
FileDataContext,
)
from great_expectations.data_context.store import (
CheckpointStore,
EvaluationParameterStore,
ExpectationsStore,
ValidationsStore,
)
from great_expectations.data_context.store.checkpoint_store import V1CheckpointStore

Check warning on line 40 in great_expectations/data_context/data_context/context_factory.py

View check run for this annotation

Codecov / codecov/patch

great_expectations/data_context/data_context/context_factory.py#L40

Added line #L40 was not covered by tests
from great_expectations.data_context.store.validation_definition_store import (
ValidationDefinitionStore,
)
Expand Down Expand Up @@ -97,8 +97,8 @@
def get_expectations_store(self) -> ExpectationsStore:
return self._project.expectations_store

def get_checkpoints_store(self) -> CheckpointStore:
return self._project.checkpoint_store
def get_checkpoints_store(self) -> V1CheckpointStore:
return self._project.v1_checkpoint_store

def get_validations_store(self) -> ValidationsStore:
return self._project.validations_store
Expand Down
8 changes: 8 additions & 0 deletions great_expectations/data_context/store/checkpoint_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,3 +293,11 @@ def _add(self, key: DataContextKey, value: V1Checkpoint, **kwargs):
if not self.cloud_mode:
value.id = str(uuid.uuid4())
return super()._add(key=key, value=value, **kwargs)

@override
def _update(self, key: DataContextKey, value: V1Checkpoint, **kwargs):
try:
super()._update(key=key, value=value, **kwargs)
except gx_exceptions.StoreBackendError as e:
name = key.to_tuple()[0]
raise ValueError(f"Could not find existing Checkpoint '{name}' to update") from e
Copy link
Member

Choose a reason for hiding this comment

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

isn't this the same issue as before? There are other valid errors -- such as validation -- Cloud backend can throw, so this error message is too specific.

18 changes: 18 additions & 0 deletions tests/checkpoint/test_v1_checkpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import great_expectations as gx
from great_expectations import expectations as gxe
from great_expectations import set_context
from great_expectations.checkpoint.actions import (
MicrosoftTeamsNotificationAction,
SlackNotificationAction,
Expand All @@ -26,6 +27,7 @@
from great_expectations.core.result_format import ResultFormat
from great_expectations.core.run_identifier import RunIdentifier
from great_expectations.core.validation_definition import ValidationDefinition
from great_expectations.data_context.data_context.abstract_data_context import AbstractDataContext
from great_expectations.data_context.data_context.ephemeral_data_context import (
EphemeralDataContext,
)
Expand All @@ -47,6 +49,22 @@ def test_checkpoint_no_validation_definitions_raises_error():
assert "Checkpoint must contain at least one validation definition" in str(e.value)


@pytest.mark.unit
def test_checkpoint_save_success(mocker: pytest.MockFixture):
context = mocker.Mock(spec=AbstractDataContext)
set_context(project=context)

checkpoint = Checkpoint(
name="my_checkpoint",
validation_definitions=[mocker.Mock(spec=ValidationDefinition)],
actions=[],
)
store_key = context.v1_checkpoint_store.get_key.return_value
checkpoint.save()

context.v1_checkpoint_store.update.assert_called_once_with(key=store_key, value=checkpoint)


@pytest.fixture
def slack_action():
return SlackNotificationAction(
Expand Down
36 changes: 11 additions & 25 deletions tests/core/factory/test_checkpoint_factory.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import pytest
from pytest_mock import MockerFixture

from great_expectations import set_context
from great_expectations.checkpoint.v1_checkpoint import Checkpoint
from great_expectations.core.expectation_suite import ExpectationSuite
from great_expectations.core.factory.checkpoint_factory import CheckpointFactory
from great_expectations.core.validation_definition import ValidationDefinition
from great_expectations.data_context import AbstractDataContext
from great_expectations.data_context.store.checkpoint_store import (
V1CheckpointStore as CheckpointStore,
)
Expand All @@ -24,9 +22,7 @@ def test_checkpoint_factory_get_uses_store_get(mocker: MockerFixture):
name=name, validation_definitions=[mocker.Mock(spec=ValidationDefinition)], actions=[]
)
store.get.return_value = checkpoint
context = mocker.MagicMock(spec=AbstractDataContext)
factory = CheckpointFactory(store=store, context=context)
set_context(context)
factory = CheckpointFactory(store=store)

# Act
result = factory.get(name=name)
Expand All @@ -47,9 +43,7 @@ def test_checkpoint_factory_get_raises_error_on_missing_key(mocker: MockerFixtur
name=name, validation_definitions=[mocker.Mock(spec=ValidationDefinition)], actions=[]
)
store.get.return_value = checkpoint
context = mocker.MagicMock(spec=AbstractDataContext)
factory = CheckpointFactory(store=store, context=context)
set_context(context)
factory = CheckpointFactory(store=store)

# Act
with pytest.raises(DataContextError, match=f"Checkpoint with name {name} was not found."):
Expand All @@ -67,9 +61,7 @@ def test_checkpoint_factory_add_uses_store_add(mocker: MockerFixture):
store.has_key.return_value = False
key = store.get_key.return_value
store.get.return_value = None
context = mocker.MagicMock(spec=AbstractDataContext)
set_context(context)
factory = CheckpointFactory(store=store, context=context)
factory = CheckpointFactory(store=store)
checkpoint = Checkpoint(
name=name, validation_definitions=[mocker.Mock(spec=ValidationDefinition)], actions=[]
)
Expand All @@ -88,9 +80,7 @@ def test_checkpoint_factory_add_raises_for_duplicate_key(mocker: MockerFixture):
name = "test-checkpoint"
store = mocker.MagicMock(spec=CheckpointStore)
store.has_key.return_value = True
context = mocker.MagicMock(spec=AbstractDataContext)
factory = CheckpointFactory(store=store, context=context)
set_context(context)
factory = CheckpointFactory(store=store)
checkpoint = Checkpoint(
name=name, validation_definitions=[mocker.Mock(spec=ValidationDefinition)], actions=[]
)
Expand All @@ -113,9 +103,7 @@ def test_checkpoint_factory_delete_uses_store_remove_key(mocker: MockerFixture):
store = mocker.MagicMock(spec=CheckpointStore)
store.has_key.return_value = True
key = store.get_key.return_value
context = mocker.MagicMock(spec=AbstractDataContext)
factory = CheckpointFactory(store=store, context=context)
set_context(context)
factory = CheckpointFactory(store=store)
checkpoint = Checkpoint(
name=name, validation_definitions=[mocker.Mock(spec=ValidationDefinition)], actions=[]
)
Expand All @@ -135,9 +123,7 @@ def test_checkpoint_factory_delete_raises_for_missing_checkpoint(mocker: MockerF
name = "test-checkpoint"
store = mocker.MagicMock(spec=CheckpointStore)
store.has_key.return_value = False
context = mocker.MagicMock(spec=AbstractDataContext)
factory = CheckpointFactory(store=store, context=context)
set_context(context)
factory = CheckpointFactory(store=store)
checkpoint = Checkpoint(
name=name, validation_definitions=[mocker.Mock(spec=ValidationDefinition)], actions=[]
)
Expand All @@ -164,16 +150,16 @@ def test_checkpoint_factory_is_initialized_with_context_cloud(empty_cloud_data_c


@pytest.mark.filesystem
def test_checkpoint_factory_add_success_filesystem(empty_data_context, mocker: MockerFixture):
_test_checkpoint_factory_add_success(empty_data_context, mocker)
def test_checkpoint_factory_add_success_filesystem(empty_data_context):
_test_checkpoint_factory_add_success(empty_data_context)


@pytest.mark.cloud
def test_checkpoint_factory_add_success_cloud(empty_cloud_context_fluent, mocker: MockerFixture):
_test_checkpoint_factory_add_success(empty_cloud_context_fluent, mocker)
def test_checkpoint_factory_add_success_cloud(empty_cloud_context_fluent):
_test_checkpoint_factory_add_success(empty_cloud_context_fluent)


def _test_checkpoint_factory_add_success(context, mocker):
def _test_checkpoint_factory_add_success(context):
# Arrange
name = "test-checkpoint"
ds = context.sources.add_pandas("my_datasource")
Expand Down
14 changes: 13 additions & 1 deletion tests/data_context/store/test_v1_checkpoint_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ def mock_checkpoint_dict(mocker, mock_checkpoint_json: dict) -> dict:
def checkpoint(
mocker: MockerFixture, mock_checkpoint_json: dict, mock_checkpoint_dict: dict
) -> V1CheckpointStore:
cp = mocker.Mock(spec=Checkpoint, id=None)
cp = mocker.Mock(spec=Checkpoint, name="my_checkpoint", id=None)
cp.json.return_value = json.dumps(mock_checkpoint_json)
cp.dict.return_value = mock_checkpoint_dict
return cp
Expand Down Expand Up @@ -293,3 +293,15 @@ def test_gx_cloud_response_json_to_object_dict_success(response_json: dict):
def test_gx_cloud_response_json_to_object_dict_failure(response_json: dict, error_substring: str):
with pytest.raises(ValueError, match=f"{error_substring}*."):
V1CheckpointStore.gx_cloud_response_json_to_object_dict(response_json)


@pytest.mark.unit
def test_update_failure_wraps_store_backend_error(
ephemeral_store: V1CheckpointStore, checkpoint: Checkpoint
):
key = ephemeral_store.get_key(name="my_nonexistant_checkpoint")

with pytest.raises(ValueError) as e:
ephemeral_store.update(key=key, value=checkpoint)

assert "Could not find existing Checkpoint" in str(e.value)
2 changes: 1 addition & 1 deletion tests/data_context/test_data_context_state_management.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,10 @@ def __init__(
) -> None:
# expectation store is required for initializing the base DataContext
self._expectations_store = ExpectationsStoreSpy()
self._checkpoint_store = CheckpointStoreSpy()
super().__init__(project_config)
self.save_count = 0
self._datasource_store = DatasourceStoreSpy()
self._checkpoint_store = CheckpointStoreSpy()

@property
def datasource_store(self):
Expand Down
2 changes: 1 addition & 1 deletion tests/data_context/test_project_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def test_get_checkpoints_store_success(self):

store = project_manager.get_checkpoints_store()

assert store == context.checkpoint_store
assert store == context.v1_checkpoint_store

@pytest.mark.unit
def test_get_checkpoints_store_fails_without_context(self):
Expand Down