[FEATURE] Add Checkpoint.run analytics #10382
Conversation
joshua-stauffer
changed the title
✅ Deploy Preview for niobium-lead-7998 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## develop #10382 +/- ##
========================================
Coverage 79.86% 79.87%
========================================
Files 460 460
Lines 39855 39871 +16
========================================
+ Hits 31830 31845 +15
- Misses 8025 8026 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -6,6 +6,7 @@ | |||
| DATA_CONTEXT_INITIALIZED: Action = Action("data_context.initialized") | |||
| CHECKPOINT_CREATED: Action = Action("checkpoint.created") | |||
| CHECKPOINT_DELETED: Action = Action("checkpoint.deleted") | |||
| CHECKPOINT_RAN: Action = Action("checkpoint.ran") | |||
There was a problem hiding this comment.
every analytics action is named in the past tense, so checkpoint.ran is consistent.
| @dataclass | ||
| class CheckpointRanEvent(_CheckpointEvent): | ||
| _allowed_actions: ClassVar[List[Action]] = [CHECKPOINT_RAN] | ||
|
|
||
| def __init__(self, checkpoint_id: str | None = None): | ||
| super().__init__( | ||
| action=CHECKPOINT_RAN, | ||
| checkpoint_id=checkpoint_id, | ||
| ) |
There was a problem hiding this comment.
Do we want additional values (like the validation definition ids)? Are we aligned with Tetris on this?
tests/checkpoint/test_checkpoint.py
Outdated
| ): | ||
| context = mocker.Mock(spec=AbstractDataContext) | ||
| set_context(project=context) | ||
| mocker.patch("great_expectations.checkpoint.checkpoint.submit_analytics_event") |
There was a problem hiding this comment.
Do we have to mock this? Or can we disable usage stats such that we don't even care about this call? I'm pretty confident that CI won't send events like this
There was a problem hiding this comment.
thanks! i was just trying to do due diligence to not send events from tests, whether from CI or when triggered by a developer, but I can remove this is it isn't necessary.
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!