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
[BUGFIX] Fix copying of CheckpointConfig for substitution and printing purposes #3759
Changes from all commits
83b3e2d
8ae9055
8f4860f
c1768f5
bbe0734
3843b93
22c1fc9
ba6d76d
11141e8
a2b17d3
ae7b7ff
2d3b8ab
4f87a42
de7db46
e769faa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,12 @@ | ||
import json | ||
from copy import deepcopy | ||
from typing import Dict, List, Optional, Union | ||
from typing import Any, Dict, List, Optional, Tuple, Union | ||
|
||
from great_expectations.core.batch import BatchRequest | ||
from great_expectations.core.batch import ( | ||
delete_runtime_parameters_batch_data_references_from_config, | ||
get_runtime_parameters_batch_data_references_from_config, | ||
restore_runtime_parameters_batch_data_references_into_config, | ||
) | ||
from great_expectations.core.expectation_validation_result import ( | ||
ExpectationSuiteValidationResult, | ||
) | ||
|
@@ -284,97 +288,28 @@ def _list_validation_statistics(self) -> Dict[ValidationResultIdentifier, dict]: | |
} | ||
return self._validation_statistics | ||
|
||
def to_json_dict(self): | ||
json_dict = {} | ||
batch_data_list = [] | ||
batch_data = None | ||
if len(self.checkpoint_config.validations) > 0: | ||
for val in self.checkpoint_config["validations"]: | ||
if ( | ||
val.get("batch_request") is not None | ||
and val["batch_request"].get("runtime_parameters") is not None | ||
and val["batch_request"]["runtime_parameters"].get("batch_data") | ||
is not None | ||
): | ||
batch_data_list.append( | ||
val["batch_request"]["runtime_parameters"].pop("batch_data") | ||
) | ||
else: | ||
batch_data_list.append(None) | ||
elif self.checkpoint_config.batch_request is not None: | ||
if ( | ||
self.checkpoint_config.batch_request is not None | ||
and self.checkpoint_config.batch_request.get("runtime_parameters") | ||
is not None | ||
and self.checkpoint_config.batch_request["runtime_parameters"].get( | ||
"batch_data" | ||
) | ||
is not None | ||
): | ||
batch_data = self.checkpoint_config["batch_request"][ | ||
"runtime_parameters" | ||
].pop("batch_data") | ||
|
||
json_dict = checkpointResultSchema.dump(self) | ||
if len(batch_data_list) > 0: | ||
for idx, val in enumerate(json_dict["checkpoint_config"]["validations"]): | ||
if batch_data_list[idx] is not None: | ||
self.checkpoint_config["validations"][idx]["batch_request"][ | ||
"runtime_parameters" | ||
]["batch_data"] = json_dict["checkpoint_config"]["validations"][ | ||
idx | ||
][ | ||
"batch_request" | ||
][ | ||
"runtime_parameters" | ||
][ | ||
"batch_data" | ||
] = batch_data_list[ | ||
idx | ||
] | ||
elif batch_data is not None: | ||
self.checkpoint_config["batch_request"]["runtime_parameters"][ | ||
"batch_data" | ||
] = json_dict["checkpoint_config"]["batch_request"]["runtime_parameters"][ | ||
"batch_data" | ||
] = batch_data | ||
|
||
return json_dict | ||
def to_json_dict(self) -> dict: | ||
return checkpointResultSchema.dump(self) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❤️ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 |
||
|
||
def __repr__(self): | ||
serializeable_dict = self.to_json_dict() | ||
if len(serializeable_dict["checkpoint_config"].get("validations")) > 0: | ||
for val in serializeable_dict["checkpoint_config"]["validations"]: | ||
if (val["batch_request"].get("runtime_parameters") is not None) and ( | ||
val["batch_request"]["runtime_parameters"].get("batch_data") | ||
is not None | ||
): | ||
val["batch_request"]["runtime_parameters"]["batch_data"] = str( | ||
type(val["batch_request"]["runtime_parameters"]["batch_data"]) | ||
) | ||
|
||
if serializeable_dict.get("batch_request") is not None: | ||
if ( | ||
serializeable_dict["checkpoint_config"]["batch_request"].get( | ||
"runtime_parameters" | ||
) | ||
is not None | ||
) and ( | ||
serializeable_dict["checkpoint_config"]["batch_request"][ | ||
"runtime_parameters" | ||
].get("batch_data") | ||
is not None | ||
): | ||
serializeable_dict["checkpoint_config"]["batch_request"][ | ||
"runtime_parameters" | ||
]["batch_data"] = str( | ||
type( | ||
serializeable_dict["checkpoint_config"]["batch_request"][ | ||
"runtime_parameters" | ||
]["batch_data"] | ||
) | ||
) | ||
|
||
batch_data_references: Tuple[ | ||
Optional[Any], Optional[List[Any]] | ||
] = get_runtime_parameters_batch_data_references_from_config( | ||
alexsherstinsky marked this conversation as resolved.
Show resolved
Hide resolved
|
||
config=self["checkpoint_config"] | ||
) | ||
delete_runtime_parameters_batch_data_references_from_config( | ||
config=self["checkpoint_config"] | ||
) | ||
serializeable_dict: dict = self.to_json_dict() | ||
restore_runtime_parameters_batch_data_references_into_config( | ||
config=self["checkpoint_config"], | ||
batch_data_references=batch_data_references, | ||
) | ||
restore_runtime_parameters_batch_data_references_into_config( | ||
config=serializeable_dict["checkpoint_config"], | ||
batch_data_references=batch_data_references, | ||
replace_value_with_type_string=True, | ||
) | ||
return json.dumps(serializeable_dict, indent=2) | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,14 +5,15 @@ | |
import ssl | ||
from email.mime.multipart import MIMEMultipart | ||
from email.mime.text import MIMEText | ||
from typing import Optional | ||
from typing import Optional, Union | ||
|
||
import requests | ||
|
||
import great_expectations.exceptions as ge_exceptions | ||
from great_expectations.core.batch import BatchRequest, RuntimeBatchRequest | ||
from great_expectations.core.util import nested_update | ||
from great_expectations.data_context.types.base import CheckpointConfig | ||
from great_expectations.util import filter_properties_dict | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
@@ -207,26 +208,11 @@ def send_email( | |
# TODO: <Alex>A common utility function should be factored out from DataContext.get_batch_list() for any purpose.</Alex> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this TODO still valid? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @donaldheppner Unfortunately, this comment is still valid -- we have not had a chance to do the work. I would like to leave it for now and remove only after we had a chance to go through the backlog formally. Thank you. |
||
def get_runtime_batch_request( | ||
substituted_runtime_config: CheckpointConfig, | ||
validation_batch_request: Optional[dict] = None, | ||
validation_batch_request: Optional[Union[dict, BatchRequest]] = None, | ||
ge_cloud_mode: bool = False, | ||
) -> Optional[BatchRequest]: | ||
runtime_config_batch_request = substituted_runtime_config.batch_request | ||
|
||
if ( | ||
( | ||
runtime_config_batch_request is not None | ||
and "runtime_parameters" in runtime_config_batch_request | ||
) | ||
or ( | ||
validation_batch_request is not None | ||
and "runtime_parameters" in validation_batch_request | ||
) | ||
or (isinstance(validation_batch_request, RuntimeBatchRequest)) | ||
): | ||
batch_request_class = RuntimeBatchRequest | ||
else: | ||
batch_request_class = BatchRequest | ||
|
||
if runtime_config_batch_request is None and validation_batch_request is None: | ||
return None | ||
|
||
|
@@ -236,6 +222,16 @@ def get_runtime_batch_request( | |
if validation_batch_request is None: | ||
validation_batch_request = {} | ||
|
||
effective_batch_request: dict = dict( | ||
**runtime_config_batch_request, **validation_batch_request | ||
) | ||
if "runtime_parameters" in effective_batch_request or isinstance( | ||
validation_batch_request, RuntimeBatchRequest | ||
): | ||
batch_request_class = RuntimeBatchRequest | ||
else: | ||
batch_request_class = BatchRequest | ||
|
||
Comment on lines
+225
to
+234
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Much better! |
||
if ( | ||
validation_batch_request.get("runtime_parameters") is not None | ||
and validation_batch_request["runtime_parameters"].get("batch_data") is not None | ||
|
@@ -265,6 +261,13 @@ def get_runtime_batch_request( | |
batch_identifiers["timestamp"] = str(datetime.datetime.now()) | ||
runtime_batch_request_dict["batch_identifiers"] = batch_identifiers | ||
|
||
filter_properties_dict( | ||
properties=runtime_batch_request_dict, | ||
keep_fields=batch_request_class.field_names, | ||
clean_nulls=False, | ||
inplace=True, | ||
) | ||
|
||
return batch_request_class(**runtime_batch_request_dict) | ||
|
||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why was this line removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it was just moved to the bottom (new line 336)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@donaldheppner I simply moved it to a "more logical place" congruent with the order these arguments were supplied on input to the method.