-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
S3 Asset e2e annotated with TODOs #9876
base: develop
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for niobium-lead-7998 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
for more information, see https://pre-commit.ci
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #9876 +/- ##
============================================
- Coverage 78.61% 64.21% -14.41%
============================================
Files 484 493 +9
Lines 42394 42537 +143
============================================
- Hits 33330 27316 -6014
- Misses 9064 15221 +6157
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
docs/docusaurus/docs/snippets/e2e_pandas_s3/07-run-checkpoint.py
Outdated
Show resolved
Hide resolved
docs/docusaurus/docs/snippets/e2e_pandas_s3/01-create-expectations-interactively.py
Outdated
Show resolved
Hide resolved
for more information, see https://pre-commit.ci
|
||
try: | ||
suite = context.suites.get("project_name") | ||
# TODO: error will change to ResourceNotFoundError |
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.
# TODO: error will change to ResourceNotFoundError | ||
except DataContextError: | ||
# TODO: will change to: | ||
# suite = context.suites.add(name="project_name") |
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.
"s3://nyc-tlc/trip data/yellow_tripdata_2019-01.parquet" | ||
) | ||
|
||
# TODO: column_index will not be required |
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.
completed in #9860
try: | ||
data_source = context.datasources["project_name"] | ||
# TODO: this will be updated to become | ||
# data_source = context.data_sources.get("project_name") |
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.
import great_expectations.expectations as gxe | ||
from great_expectations import get_context | ||
|
||
# TODO: will become from great_expectations import get_context, ExpectationSuite |
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.
Should be fixed as a followup to https://greatexpectations.atlassian.net/browse/V1-67
# TODO: will become except ResourceNotFoundError: | ||
except DataContextError: | ||
# TODO: will become | ||
# validation_definition = context.validation_definitions.add( |
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.
batch_parameters={"year": "2020", "month": "04"} | ||
) | ||
|
||
# TODO: This should only run on the latest batch, or it should fail entirely with an error that |
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.
This should be running on the latest batch already, right? Is this just about how we load data from all the assets? https://greatexpectations.atlassian.net/browse/V1-292 should tackle that.
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.
and https://greatexpectations.atlassian.net/browse/V1-327 as a backup
context = get_context(project_root_dir="./") | ||
# NOTE: It is critical to pass the batch_parameters to the run method, otherwise the validation stall | ||
# by trying to read all the data. We will have a fix in place before the final release. | ||
# TODO: Implement fix for above issue |
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.
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.
and https://greatexpectations.atlassian.net/browse/V1-327 as a backup
@@ -0,0 +1,31 @@ | |||
from great_expectations import get_context | |||
|
|||
# TODO: will become from great_expectations import Checkpoint |
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.
should be handled in followup to https://greatexpectations.atlassian.net/browse/V1-67
validation_definitions=[context.validation_definitions.get("my_project")], | ||
actions=[ | ||
SlackNotificationAction( | ||
# TODO: config variable substitution not working |
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.
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!