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

[MAINTENANCE] Column Descriptive Metrics: Return id from Data Store #8667

Merged
merged 7 commits into from Sep 6, 2023

Conversation

anthonyburdi
Copy link
Member

@anthonyburdi anthonyburdi commented Sep 5, 2023

The ID is needed of the metric_run to populate the CreatedResource of the ActionResult. The rest of the changes will first be implemented in the cloud repo https://github.com/great-expectations/cloud

Eventually we will likely want to rehydrate the MetricRun & Metrics within OSS (e.g. if used as a metric store) however for the Column Descriptive Metrics project we don't have a use currently for the rehydrated objects. In the spirit of YAGNI / not overbuilding yet this PR only returns the id of the created MetricRun instead of using the response payload to build a MetricRun object with corresponding Metrics. The returned ID is used in the agent to populate the CreatedResource. Note: if/when we implement creating the MetricRun & Metric objects in OSS, we will also need to build deserialization logic to use the appropriate metric type for validation.

  • 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 black + ruff)
  • Appropriate tests and docs have been updated

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!

@netlify
Copy link

netlify bot commented Sep 5, 2023

Deploy Preview for niobium-lead-7998 canceled.

Name Link
🔨 Latest commit f75c3f4
🔍 Latest deploy log https://app.netlify.com/sites/niobium-lead-7998/deploys/64f7bf5a3e67b00008f8cd0e

@ghost
Copy link

ghost commented Sep 5, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

response.raise_for_status()

response_json = response.json()
return uuid.UUID(response_json["data"]["attributes"]["id"])
Copy link
Contributor

Choose a reason for hiding this comment

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

ok this is a change that makes a lot of sense

@anthonyburdi anthonyburdi enabled auto-merge (squash) September 6, 2023 00:04
@anthonyburdi anthonyburdi merged commit 8d794f9 into develop Sep 6, 2023
53 checks passed
@anthonyburdi anthonyburdi deleted the m/dx-598/dx-640/oss-agent-action branch September 6, 2023 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants