[MAINTENANCE] Migrate OpsgenieNotificationAction
#9716
Conversation
✅ Deploy Preview for niobium-lead-7998 canceled.
|
…_expectations into m/v1-251/opsgenie_action
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #9716 +/- ##
===========================================
+ Coverage 82.62% 82.64% +0.01%
===========================================
Files 512 512
Lines 46532 46581 +49
===========================================
+ Hits 38446 38495 +49
Misses 8086 8086
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| assets_validated_by_batch_id[batch_id]["validation_results"].append( # type: ignore[union-attr] | ||
| validation_result | ||
| ) | ||
| assets_validated_by_batch_id[batch_id]["expectation_suite_names"].append( | ||
| assets_validated_by_batch_id[batch_id]["expectation_suite_names"].append( # type: ignore[union-attr] |
There was a problem hiding this comment.
This is the old checkpoint result (not sure why mypy is mad about it now). We're using a new Pydantic type so I think we can ignore these?
| @@ -514,7 +514,7 @@ def __init__( # noqa: PLR0913 | |||
| suite_name: str, | |||
| evaluation_parameters: Optional[dict] = None, | |||
| statistics: Optional[dict] = None, | |||
| meta: Optional[ExpectationSuiteValidationResult | dict] = None, | |||
| meta: Optional[ExpectationSuiteValidationResultMeta | dict] = None, | |||
There was a problem hiding this comment.
This has been wrong for a while (thank you @tyler-hoffman for pointing it out). Some minor changes in this PR to adhere to the proper type
| @property | ||
| def asset_name(self) -> str | None: | ||
| if "active_batch_definition" in self.meta: | ||
| return self.meta["active_batch_definition"].data_asset_name | ||
| return None |
There was a problem hiding this comment.
Would like to make this a top-level property (might write a ticket around it) but not pressing
| checkpoint_name = checkpoint_result.checkpoint_config.name | ||
|
|
||
| if ( | ||
| self.notify_on == "all" |
There was a problem hiding this comment.
I was thinking about this in the other PR but didn't mention it because it was just reusing existing code: maybe it's just me but I find these longer lines of or / and hard to parse in python. What about making intermediate variables for the and parts, and then just having this condition do the ors if that makes sense.
| @@ -606,6 +606,41 @@ def _validate_renderer(cls, renderer: dict | OpsgenieRenderer) -> OpsgenieRender | |||
| renderer = _renderer | |||
| return renderer | |||
|
|
|||
| @override | |||
| def v1_run(self, checkpoint_result: CheckpointResult) -> dict: | |||
There was a problem hiding this comment.
Lots of overlap with _run. I might DRY this up, but if we can remove _run in the near term, I'm less stressed about it.
There was a problem hiding this comment.
Legacy _run will be removed shortly!
| @@ -1044,19 +1080,8 @@ def _run( # type: ignore[override] # signature does not match parent | |||
| checkpoint_identifier=None, | |||
| **kwargs, | |||
| ): | |||
| suite_name: str = validation_result_suite.meta["expectation_suite_name"] | |||
There was a problem hiding this comment.
I'm confused on this change. This is a separate action, right?
There was a problem hiding this comment.
Yup but mypy kept flagging it as an issue
There was a problem hiding this comment.
Probably because of the change to meta below
| checkpoint_result=checkpoint_result, text_blocks=text_blocks | ||
| ) | ||
|
|
||
| def _v1_render(self, run_result: ExpectationSuiteValidationResult) -> str: |
There was a problem hiding this comment.
This should be named to indicate it's just about a single result; first look at this method and the above made me question why one was public and one was private, and why they would be different.
Opsgenie renderer and action need to support V1's checkpoint result API
invoke lint(usesruff format+ruff check)For more information about contributing, see Contribute.
After you submit your PR, keep the page open and monitor the statuses of the various checks made by our continuous integration process at the bottom of the page. Please fix any issues that come up and reach out on Slack if you need help. Thanks for contributing!