-
Notifications
You must be signed in to change notification settings - Fork 1.2k
mgmt: add back serviceip in ManagementServerResponse #10891
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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.20 #10891 +/- ##
=========================================
Coverage 16.13% 16.14%
- Complexity 13239 13241 +2
=========================================
Files 5656 5656
Lines 497579 497586 +7
Branches 60333 60333
=========================================
+ Hits 80297 80312 +15
+ Misses 408333 408324 -9
- Partials 8949 8950 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your 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.
LGTM
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 - since this fixes API regression, we should consider this as critical for 4.20.1
@weizhouapache can you also look at failing github action smoketests? |
@rohityadavcloud a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 13454 |
thanks @rohityadavcloud the remaining test failures are addressed by #10875 |
@blueorangutan test |
@weizhouapache a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
@weizhouapache , can we somehow mark it as deprecated? (ltgm, btw) |
@DaanHoogland |
I think it would be good to add it to the description that using ipaddress instead is advised. |
that's already added. @DaanHoogland I added |
sorry, reviewed from the wild (not at desk)
great, I don’t think we can do better. |
[SF] Trillian test result (tid-13359)
|
* mgmt: add back serviceip in ManagementServerResponse * api: add Deprecated annotation to serviceip
Description
The PR #9773 changed the
serviceip
toipaddress
in the API response, which broke backward compatibility.Although a test failure has been fixed in #10501 , I think it would be better to add it back.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?