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

Run action menu displays run metric based on graphql field instead of tag #24664

Merged

Conversation

mlarose
Copy link
Contributor

@mlarose mlarose commented Sep 23, 2024

Summary & Motivation

Use the new graphql 'hasRunMetricsEnabled' field created in #24662 to condition visibility of run metrics in action menu

How I Tested These Changes

  • manual testing

Changelog

NOCHANGELOG

Copy link

github-actions bot commented Sep 23, 2024

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-4h9q92xga-elementl.vercel.app
https://mlarose-dagit-uses-graphql-run-metrics-enabled-field.core-storybook.dagster-docs.io

Built with commit fb8e0d3.
This pull request is being automatically deployed with vercel-action

@mlarose

This comment was marked as resolved.

@mlarose mlarose requested a review from hellendag September 23, 2024 17:15
@mlarose mlarose changed the title generate graphql Run action menu displays run metric based on graphql field instead of tag Sep 23, 2024
@mlarose mlarose force-pushed the mlarose/expose_enable_run_metrics_as_graphql_field branch from ec2435b to 5a113df Compare October 7, 2024 14:43
@mlarose mlarose force-pushed the mlarose/dagit-uses-graphql-run-metrics-enabled-field branch from a0b9774 to c44e05b Compare October 7, 2024 14:43
@mlarose mlarose force-pushed the mlarose/expose_enable_run_metrics_as_graphql_field branch from 5a113df to 60cdf38 Compare October 7, 2024 14:59
@mlarose mlarose force-pushed the mlarose/dagit-uses-graphql-run-metrics-enabled-field branch 2 times, most recently from 51bbd4a to fd50629 Compare October 7, 2024 15:04
@mlarose mlarose changed the base branch from mlarose/expose_enable_run_metrics_as_graphql_field to mlarose/run_metrics_hidden_tag_future_proofing October 7, 2024 15:04
@mlarose mlarose force-pushed the mlarose/run_metrics_hidden_tag_future_proofing branch from 4b18021 to 3807049 Compare October 7, 2024 18:03
@mlarose mlarose force-pushed the mlarose/dagit-uses-graphql-run-metrics-enabled-field branch 2 times, most recently from 882e669 to c916d23 Compare October 7, 2024 18:06
@mlarose mlarose marked this pull request as ready for review October 7, 2024 18:06
@mlarose mlarose force-pushed the mlarose/run_metrics_hidden_tag_future_proofing branch from 3807049 to 2351677 Compare October 8, 2024 11:45
@mlarose mlarose force-pushed the mlarose/dagit-uses-graphql-run-metrics-enabled-field branch from aad044f to 740e0f6 Compare October 8, 2024 11:45
@mlarose mlarose requested review from salazarm and gibsondan and removed request for hellendag October 8, 2024 11:46
@@ -518,6 +516,7 @@ export const RUN_ACTIONS_MENU_RUN_FRAGMENT = gql`
repositoryName
repositoryLocationName
}
hasRunMetricsEnabled
Copy link
Member

Choose a reason for hiding this comment

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

uber nit, one could call this just 'runMetricsEnabled' ('has' and 'enabled' are arguably redundant)

@mlarose mlarose force-pushed the mlarose/run_metrics_hidden_tag_future_proofing branch from 2351677 to 710652d Compare October 8, 2024 15:15
@mlarose mlarose force-pushed the mlarose/dagit-uses-graphql-run-metrics-enabled-field branch from 5d35f90 to d73e45a Compare October 8, 2024 15:15
Copy link
Contributor Author

mlarose commented Oct 8, 2024

Merge activity

  • Oct 8, 2:05 PM EDT: @mlarose started a stack merge that includes this pull request via Graphite.
  • Oct 8, 2:22 PM EDT: Graphite rebased this pull request as part of a merge.
  • Oct 8, 2:23 PM EDT: @mlarose merged this pull request with Graphite.

@mlarose mlarose changed the base branch from mlarose/run_metrics_hidden_tag_future_proofing to graphite-base/24664 October 8, 2024 18:17
@mlarose mlarose changed the base branch from graphite-base/24664 to master October 8, 2024 18:19
Use the new GraphQL hasRunMetricsEnabled field created in #24662 to condition visibility of run metrics in action menu
@mlarose mlarose force-pushed the mlarose/dagit-uses-graphql-run-metrics-enabled-field branch from d73e45a to fb8e0d3 Compare October 8, 2024 18:21
@mlarose mlarose merged commit 27cc76b into master Oct 8, 2024
1 of 2 checks passed
@mlarose mlarose deleted the mlarose/dagit-uses-graphql-run-metrics-enabled-field branch October 8, 2024 18:23
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