-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[ui] Append a dagster/from_ui tag to every run launched from the UI #29156
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
Conversation
Deploy preview for dagit-core-storybook ready! ✅ Preview Built with commit 33df4ee. |
const finalized = { | ||
...variables, | ||
executionParamsList: | ||
variables.executionParamsList instanceof Array |
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.
Oh nice did not know about this actually!
This is a small change but it applies to: - Job execution - Job backfills - Asset graph materialization - Asset graph backfills - Run re-execution (via extraTags) - Backfill re-execution - Multiple-run launching - Evaluating schedules - Sensor dry runs
4b82b8a
to
33df4ee
Compare
|
||
const result = (await launchMultipleRuns({variables})).data?.launchMultipleRuns; | ||
const finalized = { | ||
...variables, | ||
executionParamsList: Array.isArray(variables.executionParamsList) | ||
? variables.executionParamsList.map(paramsWithUIExecutionTags) | ||
: paramsWithUIExecutionTags(variables.executionParamsList), | ||
}; |
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 appears to be an inconsistency in how executionParamsList
is handled. While the code at lines 29-31 normalizes a non-array executionParamsList
into an array for validation, lines 54-57 handle both array and non-array formats during execution. Since the GraphQL mutation expects an array of execution parameters, this could lead to unexpected behavior.
Consider standardizing the handling by ensuring executionParamsList
is always treated as an array:
executionParamsList: (Array.isArray(variables.executionParamsList)
? variables.executionParamsList
: [variables.executionParamsList]).map(paramsWithUIExecutionTags),
This approach would provide consistent parameter handling regardless of the input format.
const result = (await launchMultipleRuns({variables})).data?.launchMultipleRuns; | |
const finalized = { | |
...variables, | |
executionParamsList: Array.isArray(variables.executionParamsList) | |
? variables.executionParamsList.map(paramsWithUIExecutionTags) | |
: paramsWithUIExecutionTags(variables.executionParamsList), | |
}; | |
const finalized = { | |
...variables, | |
executionParamsList: (Array.isArray(variables.executionParamsList) | |
? variables.executionParamsList | |
: [variables.executionParamsList]).map(paramsWithUIExecutionTags), | |
}; |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
This is a small change but it applies to:
Summary & Motivation
Context: https://dagsterlabs.slack.com/archives/C058CBEL230/p1743459307738509
https://linear.app/dagster-labs/issue/FE-828/add-a-tag-that-indicates-that-a-run-was-triggered-by-a-user-via-the-ui
Allows filtering for runs created from the UI:
How I Tested These Changes
I manually tested most of the workflows above, and I also made changes to the tests I placed this code very close to the mutations to make sure it's always sent.
Changelog
[ui] Runs launched from the Dagster UI get a
dagster/from_ui = true
tag, making it easy to filter for them.