-
Notifications
You must be signed in to change notification settings - Fork 661
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
Hide generated launch plans starting with .flytegen in the UI #5949
Conversation
Signed-off-by: troychiu <[email protected]>
Signed-off-by: troychiu <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5949 +/- ##
==========================================
+ Coverage 36.84% 36.86% +0.01%
==========================================
Files 1309 1310 +1
Lines 130967 131246 +279
==========================================
+ Hits 48259 48383 +124
- Misses 78524 78665 +141
- Partials 4184 4198 +14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Lgtm but let's wait on FE to confirm expected usages
Signed-off-by: troychiu <[email protected]>
Signed-off-by: troychiu <[email protected]>
Signed-off-by: troychiu <[email protected]>
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.
thank you! do you mind updating the fully supported set of filter functions documentation here too: https://github.com/flyteorg/flyte/blob/96c467e14691eb3e956d3b21c05fb724c240fe97/docs/user_guide/concepts/control_plane/admin.rst#adding-request-filters
filters := make([]common.InlineFilter, 0) | ||
if referenceEntity == core.ResourceType_WORKFLOW { |
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.
nice, love cleaning up this code (it's also very confusing we always force a filter on workflow state even if you filter on state down below
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.
Yes that's confusing!
Signed-off-by: troychiu <[email protected]>
Why are the changes needed?
Currently, the launch plan page will show generated launch plans, which make users hard to find their launch plan. We should hide them like we do for workflows.
What changes were proposed in this pull request?
.flytegen
. I did this instead of filtering by resource state since I would like to avoid a db migration.How was this patch tested?
Screenshots
Check all the applicable boxes