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

[DOCS] Custom Actions #10772

Merged
merged 36 commits into from
Dec 18, 2024
Merged

[DOCS] Custom Actions #10772

merged 36 commits into from
Dec 18, 2024

Conversation

klavavej
Copy link
Contributor

@klavavej klavavej commented Dec 12, 2024

Resolves https://greatexpectations.atlassian.net/browse/DOC-900 according to the plan linked in that issue

Deploy Preview at https://deploy-preview-10772.docs.greatexpectations.io/docs/core/trigger_actions_based_on_results/create_a_custom_action

  • Description of PR changes above includes a link to an existing GitHub issue
  • PR title is prefixed with one of: [BUGFIX], [FEATURE], [DOCS], [MAINTENANCE], [CONTRIB]
  • Code is linted - run invoke lint (uses ruff format + ruff check)
  • Appropriate tests and docs have been updated

For more information about contributing, visit our community resources.

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!

Copy link

netlify bot commented Dec 12, 2024

Deploy Preview for niobium-lead-7998 ready!

Name Link
🔨 Latest commit 482db16
🔍 Latest deploy log https://app.netlify.com/sites/niobium-lead-7998/deploys/6762e57f2894fa00082af074
😎 Deploy Preview https://deploy-preview-10772.docs.greatexpectations.io
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

Thanks for the PR @pre-commit-ci[bot] 👋

Our GitHub actions pipelines require a user with write permissions to retry the failed jobs. We've sent a message to a maintainer to review your PR. Please be patient and we'll get back to you as soon as possible.

Copy link

codecov bot commented Dec 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.67%. Comparing base (23b1520) to head (482db16).
Report is 3 commits behind head on develop.

✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop   #10772   +/-   ##
========================================
  Coverage    80.67%   80.67%           
========================================
  Files          464      464           
  Lines        40348    40349    +1     
========================================
+ Hits         32552    32553    +1     
  Misses        7796     7796           
Flag Coverage Δ
3.10 69.96% <100.00%> (+<0.01%) ⬆️
3.10 athena or openpyxl or pyarrow or project or sqlite or aws_creds ?
3.10 aws_deps ?
3.10 big ?
3.10 clickhouse ?
3.10 filesystem ?
3.10 mssql ?
3.10 mysql ?
3.10 postgresql ?
3.10 spark_connect ?
3.11 69.96% <100.00%> (+<0.01%) ⬆️
3.11 athena or openpyxl or pyarrow or project or sqlite or aws_creds ?
3.11 aws_deps ?
3.11 big ?
3.11 clickhouse ?
3.11 filesystem ?
3.11 mssql ?
3.11 mysql ?
3.11 postgresql ?
3.11 spark_connect ?
3.11 trino ?
3.12 69.96% <100.00%> (+<0.01%) ⬆️
3.12 athena or openpyxl or pyarrow or project or sqlite or aws_creds 56.38% <100.00%> (+<0.01%) ⬆️
3.12 aws_deps 46.18% <100.00%> (+<0.01%) ⬆️
3.12 big 54.72% <100.00%> (+<0.01%) ⬆️
3.12 bigquery 48.47% <100.00%> (+<0.01%) ⬆️
3.12 databricks 50.33% <100.00%> (+<0.01%) ⬆️
3.12 filesystem 62.65% <100.00%> (+<0.01%) ⬆️
3.12 mssql 51.25% <100.00%> (+<0.01%) ⬆️
3.12 mysql 51.66% <100.00%> (+<0.01%) ⬆️
3.12 postgresql 54.38% <100.00%> (+<0.01%) ⬆️
3.12 snowflake 51.06% <100.00%> (+<0.01%) ⬆️
3.12 spark 57.66% <100.00%> (+<0.01%) ⬆️
3.12 spark_connect 46.47% <100.00%> (+<0.01%) ⬆️
3.12 trino 52.21% <100.00%> (+<0.01%) ⬆️
3.9 69.98% <100.00%> (+<0.01%) ⬆️
3.9 athena or openpyxl or pyarrow or project or sqlite or aws_creds 56.38% <100.00%> (+<0.01%) ⬆️
3.9 aws_deps 46.20% <100.00%> (+<0.01%) ⬆️
3.9 big 54.73% <100.00%> (+<0.01%) ⬆️
3.9 bigquery 48.47% <100.00%> (+<0.01%) ⬆️
3.9 clickhouse 43.08% <100.00%> (+<0.01%) ⬆️
3.9 databricks 50.33% <100.00%> (+<0.01%) ⬆️
3.9 filesystem 62.66% <100.00%> (+<0.01%) ⬆️
3.9 mssql 51.23% <100.00%> (+<0.01%) ⬆️
3.9 mysql 51.64% <100.00%> (+<0.01%) ⬆️
3.9 postgresql 54.37% <100.00%> (+<0.01%) ⬆️
3.9 snowflake 51.06% <100.00%> (+<0.01%) ⬆️
3.9 spark 57.63% <100.00%> (+<0.01%) ⬆️
3.9 spark_connect 46.48% <100.00%> (+<0.01%) ⬆️
3.9 trino 52.20% <100.00%> (+<0.01%) ⬆️
cloud 0.00% <0.00%> (ø)
docs-basic 54.09% <100.00%> (+<0.01%) ⬆️
docs-creds-needed 52.98% <100.00%> (+<0.01%) ⬆️
docs-spark 52.46% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@klavavej klavavej marked this pull request as ready for review December 13, 2024 20:59

- <PrereqPythonInstalled/>.
- <PrereqGxInstalled/>.
- <PrereqPreconfiguredDataContext/>. In this guide the variable `context` is assumed to contain your Data Context.
- <PrereqValidationDefinition/>.

### Procedure
## Procedure
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note for reviewer: apologies for scope creep but I noticed some header level issues on this page when I copied it to scaffold the new page, so I went ahead and fixed the problems on this page.

<sup>6</sup> Airflow version 2.9.0+ required<br/>
<sup>3</sup> This includes configuration storage for Expectations, Checkpoints, Validation Definitions, and Validation Results.<br/>
<sup>4</sup> We support the general workflow for creating custom Actions but cannot help troubleshoot the domain-specific logic within a custom Action.<br/>
<sup>5</sup> Use `config_variables.yml`.<br/>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note for reviewer: apologies for scope creep but I went ahead and made these footnotes more consistently structured as long as I was editing this content.

@@ -0,0 +1,46 @@
"""
This is an example script for how to create a custom Action.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note for technical reviewer: this is my first time putting a new code sample under test. I did get some errors with it that I was able to resolve so I think the test is working. But, please let me know if I need to add anything else to make sure this is sufficiently tested.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm so usually these snippet files represent actual tests and I don't think we have any test logic here.

At the bottom of the file, could we instantiate an instance of this custom class and assert against its run method?

my_action = MyCustomAction(...)
result = my_action.run(...)
assert result == {"some": "info"}

You may need to create a checkpoint and that can be a little cumbersome so let me know if you want me to jump in and add these details on your behalf.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you - I would appreciate if you'd jump in and add the extra code to make this a sufficient test!

Copy link
Member

Choose a reason for hiding this comment

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

I've updated the logic to show a concrete example of what a user might want to do but feel free to modify/critique as you see fit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you!

How does this custom calculated percentage of successful expectations across all validation results compare to the success_percent info available through ExpectationSuiteValidationResult.statistics?

If these two things provide the same exact information in different ways, are there advantages to using a custom action instead of the built-in statistics?

Depending on how similar these two things are and what if any advantages there are to choosing one option over another in various scenarios, I think we should either acknowledge the similarities and tradeoffs or pick a different concrete example.

(this feedback is somewhat similar to the feedback you had for me about the email use case in the intro overlapping a bit with our built in email action.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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


from typing import Literal

from typing_extensions import override
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note for technical reviewer: I got an error about override not being defined so I added this based on other code I see in the repo. It resolved the error so I think it's right but let me know if there's anything amiss here.

@@ -70,7 +70,7 @@ To ensure the long-term quality of the GX Core codebase, we're not yet ready to
| -------------------- | ------------------ | ----- |
| CredentialStore | 🟢 Ready | |
| BatchDefinition | 🟡 Partially ready | Formerly known as splitters |
| Action | 🔴 Not ready | |
| Action | 🟢 Ready | |
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note for product reviewer Please confirm the contribution readiness status for Actions. I tried to ask about this in my docs plan but didn't get an answer so want to double check that I've got the right status.

Copy link
Member

Choose a reason for hiding this comment

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

Not the product review but LGTM from an engineering perspective - the API is stable and the steps to create custom actions are quite simple (as laid out here 🎉 )

@@ -182,10 +182,10 @@ def __new__(cls, clsname, bases, attrs):
@public_api
class ValidationAction(BaseModel, metaclass=MetaValidationAction):
"""
ValidationActions define a set of steps to be run after a validation result is produced.
Actions define a set of steps to run after a Validation Result is produced. Subclass `ValidationAction` to create a `custom Action </docs/core/trigger_actions_based_on_results/create_a_custom_action>`_.
Copy link
Contributor Author

@klavavej klavavej Dec 13, 2024

Choose a reason for hiding this comment

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

Note for reviewer: though I've written "custom" in lowercase here, it's getting capitalized in the output. Is there any way I can force "custom" to display as lowercase here? this is my first time editing a docstring for our API reference
Screenshot 2024-12-13 at 1 10 03 PM

Copy link
Member

Choose a reason for hiding this comment

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

Huh that is a little odd - I don't know the exact logic that does this but it might be ingrained in one of the docs libs we use.

Perhaps try MD syntax for hyperlinks? Not sure that's supported but worth a shot?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

MD syntax worked! 🎉

…ions/great_expectations into kml/DOC-900/customActions

:wq
@@ -87,7 +87,7 @@ def _build_renderer(config: dict) -> Renderer:
)
return renderer


@public_api
Copy link
Contributor Author

@klavavej klavavej Dec 13, 2024

Choose a reason for hiding this comment

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

Note for technical reviewer: I added this to resolve an error about too many items missing the public_api decorator
Screenshot 2024-12-13 at 1 17 32 PM

Copy link
Member

Choose a reason for hiding this comment

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

Ah amazing I was attempting to do the same thing here: #10752

Let's take in your change and then I'll update my PR to fill in any other gaps if we have them

@@ -308,7 +308,7 @@ jobs:
# We decided to exclude all external HTTP requests but the ones that under the domain greatexpectations.io
# The reason is to avoid having network errors such as pages that throw 429 after too many requests (like Github)
# and to prevent other possible errors related to user agent or lychee capturing hrefs from metadata that don't resolve to a specific page (preconnects in JS)
args: "--exclude='http.*' --include='^https://(.+\\.)?greatexpectations\\.io/' 'docs/docusaurus/build/**/*.html'"
args: "--exclude='http.*' 'docs/docusaurus/build/**/*.html'"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note for reviewers: I'm temporarily changing the link checker config to work around a bug where the link checker fails when a PR contains a link to a page that that same PR is introducing. I'll undo this change after merging this PR to restore link checking

Copy link
Member

Choose a reason for hiding this comment

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

Ah okay I have the same issue and didn't realize it was a documented bug - thanks for the call out!


Great Expectations provides [Actions for common workflows](/application_integration_support.md#integrations) such as sending emails and updating Data Docs. If these don't meet your needs, you can create a custom Action to integrate with different tools or apply custom business logic based on Validation Results. Example use cases for custom Actions include:
- Opening tickets in an issue tracker when Validation runs fail.
- Sending emails to different teams depending on which Expectations fail.
Copy link
Member

Choose a reason for hiding this comment

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

This has some overlap with our current email action right? Should we perhaps pick another example?

Copy link
Member

@cdkini cdkini left a comment

Choose a reason for hiding this comment

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

Looks really good just a minor point or two about the test file - happy to discuss just let me know!

@klavavej klavavej enabled auto-merge December 18, 2024 15:18
@klavavej klavavej added this pull request to the merge queue Dec 18, 2024
Merged via the queue into develop with commit 10d6a4d Dec 18, 2024
71 checks passed
@klavavej klavavej deleted the kml/DOC-900/customActions branch December 18, 2024 16:30
Shinnnyshinshin added a commit that referenced this pull request Dec 18, 2024
…ps://github.com/great-expectations/great_expectations into m/zelda-1183/zelda-1188/adding-databricks-types

* 'm/zelda-1183/zelda-1188/adding-databricks-types' of https://github.com/great-expectations/great_expectations:
  [MAINTENANCE] Allow `CheckpointResult` and `ActionContext` to be importable from top-level checkpoint module (#10788)
  [DOCS] Custom Actions (#10772)
  [DOCS] Remove unnecessary escape character in Expectation for Gallery (#10780)
  [MAINTENANCE] Deprecate `DataContext.add_or_update_datasource` (#10784)
Shinnnyshinshin added a commit that referenced this pull request Dec 18, 2024
…thub.com/great-expectations/great_expectations into m/zelda-1183/zelda-1188/databricks-tests

* 'm/zelda-1183/zelda-1188/databricks-tests' of https://github.com/great-expectations/great_expectations:
  [MAINTENANCE] Allow `CheckpointResult` and `ActionContext` to be importable from top-level checkpoint module (#10788)
  [DOCS] Custom Actions (#10772)
  [DOCS] Remove unnecessary escape character in Expectation for Gallery (#10780)
  [MAINTENANCE] Deprecate `DataContext.add_or_update_datasource` (#10784)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants