-
Notifications
You must be signed in to change notification settings - Fork 665
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
Add flyte-core missing priorityClassName to webhook values #4987
Add flyte-core missing priorityClassName to webhook values #4987
Conversation
27ccb07
to
5d3336a
Compare
5d3336a
to
cdca47a
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4987 +/- ##
=======================================
Coverage 58.99% 58.99%
=======================================
Files 645 645
Lines 55648 55648
=======================================
Hits 32831 32831
Misses 20222 20222
Partials 2595 2595
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, thank you, @ddl-ebrown. Could you update the PR description?
What part of the description did you need updated? |
@ddl-ebrown, it's good now. Last time I saw it, it was empty. Maybe it was Github issue. |
c1cd31e
to
d0ccb27
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.
Looks good. Thanks!
76a414f
to
48f5050
Compare
PR updated to remove conflicts, so this should be good to go. |
Looks like there's a new process for bumping the chart version number? |
- All the other pods can have their priorityClassName set except for the webhook Signed-off-by: ddl-ebrown <[email protected]>
48f5050
to
95e8294
Compare
@davidmirror-ops repushed this one as well. Hopefully can get this across the line soon. |
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.
Once again, thanks so much
Tracking issue
https://github.com/flyteorg/flyte/issues/
Why are the changes needed?
What changes were proposed in this pull request?
How was this patch tested?
The updated chart is being used to deploy Flyte in our environment. There are tests that validate that priorityClassName propagates to the webhook pod properly
Setup process
Screenshots
Check all the applicable boxes
Related PRs
Docs link