-
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
[ui] Filter hidden definitions tags #23771
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @benpankow and the rest of your teammates on Graphite |
f1492cb
to
50ffeee
Compare
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.
How does this compare so the system tags that happen on materializations? Right now we hide those from the UI by default, for example. I think the treatment should uniform between tags here and tags there.
Are you talking about this toggle? As far as I can tell that's all handled on the frontend - the backend doesn't handle system tags differently with the sole exception that we don't validate them: |
Why don't we just do the same here for now? |
At second blush I'm not convinced there's a reason to thread this through tags - see #23778 |
50ffeee
to
f73cb45
Compare
57b0401
to
4a4518a
Compare
Deploy preview for dagit-core-storybook ready! ✅ Preview Built with commit cd640da. |
bc12351
to
2e8152e
Compare
cfa198d
to
ead2042
Compare
ead2042
to
8426cf3
Compare
I'm still not convinced we need this. |
8426cf3
to
cd640da
Compare
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.
queue mgmt. I'm still not sold on supporting hidden tags like this. We should discuss.
Summary
For run tags, we identify three tag types:
dagster/
prefix.dagster/
prefixFor asset definition tags, we already use the
dagster/
prefix as a form of "namespacing", but no asset definition tags match the run-tag notion of "system tags."Still, it would be useful to have a set of asset definition tags we hide from the UI entirely for things like the kind system.
This PR introduces the
.dagster/
prefix as a hidden tag indicator for the definition tag system. These tags are filtered out on the frontend from display.Test Plan
Tested locally