-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
fix: Change applicationset generate HTTP method to avoid route conflicts #20758
fix: Change applicationset generate HTTP method to avoid route conflicts #20758
Conversation
Signed-off-by: Amit Oren <[email protected]>
❌ Preview Environment deleted from BunnyshellAvailable commands (reply to this comment):
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #20758 +/- ##
==========================================
- Coverage 55.12% 53.29% -1.83%
==========================================
Files 324 337 +13
Lines 55256 57055 +1799
==========================================
- Hits 30458 30406 -52
- Misses 22184 24012 +1828
- Partials 2614 2637 +23 ☔ View full report in Codecov by Sentry. |
Thanks @andrii-korotkov-verkada, is there someone we can ping for further approval? |
I'll bring up the PRs marked with ready-for-review on the contributors meeting, hope they can take a look soon. |
Hey @andrii-korotkov-verkada, sorry for bothering, but as it's been a few contributors meetings since I'd like to ping this again |
I've put it as a my top item for the next meeting. |
Co-authored-by: Alexandre Gaudreault <[email protected]> Signed-off-by: Amit Oren <[email protected]>
Signed-off-by: Amit Oren <[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.
LGTM
/cherry-pick release-2.14 |
…cts (#20758) * Change applicationset generate HTTP method to avoid route conflicts Signed-off-by: Amit Oren <[email protected]> * Update server/applicationset/applicationset.proto Co-authored-by: Alexandre Gaudreault <[email protected]> Signed-off-by: Amit Oren <[email protected]> * Codegen Signed-off-by: Amit Oren <[email protected]> --------- Signed-off-by: Amit Oren <[email protected]> Signed-off-by: Amit Oren <[email protected]> Co-authored-by: Alexandre Gaudreault <[email protected]>
/cherry-pick release-2.13 |
…cts (#20758) * Change applicationset generate HTTP method to avoid route conflicts Signed-off-by: Amit Oren <[email protected]> * Update server/applicationset/applicationset.proto Co-authored-by: Alexandre Gaudreault <[email protected]> Signed-off-by: Amit Oren <[email protected]> * Codegen Signed-off-by: Amit Oren <[email protected]> --------- Signed-off-by: Amit Oren <[email protected]> Signed-off-by: Amit Oren <[email protected]> Co-authored-by: Alexandre Gaudreault <[email protected]>
…cts (#20758) (#21299) * Change applicationset generate HTTP method to avoid route conflicts * Update server/applicationset/applicationset.proto * Codegen --------- Signed-off-by: Amit Oren <[email protected]> Signed-off-by: Amit Oren <[email protected]> Co-authored-by: Amit Oren <[email protected]> Co-authored-by: Alexandre Gaudreault <[email protected]>
Closes #20757
Fixes HTTP route conflict introduced in 3a5b653
I'm not sure if
PUT
is the correct semantic we want here, however because/api/v1/applicationsets/{name}
is already taken I wasn't sure to which route to change this to. Open to change to whatever.If possible, would appreciate this as a hotfix to 2.13
Checklist: