-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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: Proper explanation for the global setting to avoid ambiguity #10042
base: 4.20
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.20 #10042 +/- ##
============================================
+ Coverage 15.99% 16.00% +0.01%
- Complexity 13076 13081 +5
============================================
Files 5649 5649
Lines 495612 495978 +366
Branches 59995 60134 +139
============================================
+ Hits 79249 79364 +115
- Misses 407514 407769 +255
+ Partials 8849 8845 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
18cfe56
to
425fc95
Compare
@blueorangutan package |
@Pearl1594 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 11714 |
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 mostly good, needs testing
engine/schema/src/main/resources/META-INF/db/schema-41910to42000.sql
Outdated
Show resolved
Hide resolved
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.
@Pearl1594 I think if we just do the following change then it should be good enough. In my opinion, global config is largely okay.
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
…me-ambiguous-globalsetting
@nvazquez Apologies for the confusion, I had put the wrong name in the PR description - the global setting name has been renamed to |
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java
Outdated
Show resolved
Hide resolved
[SF] Trillian test result (tid-12380)
|
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.
@Pearl1594 I guess we're in somewhat agreement that changes should be limited to ui/src/views/infra/HostEnableDisable.vue
Can you please revert other changes and rename the PR title?
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.
Thanks @Pearl1594 LGTM - just a few non-blocking comments to keep the changes to minimal
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java
Outdated
Show resolved
Hide resolved
server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
Outdated
Show resolved
Hide resolved
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.
minor suggestions, otherwise code lgtm
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java
Outdated
Show resolved
Hide resolved
server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
Outdated
Show resolved
Hide resolved
server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
Outdated
Show resolved
Hide resolved
@blueorangutan package |
@Pearl1594 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 12448 |
@blueorangutan test |
@Pearl1594 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests |
[SF] Trillian test result (tid-12412)
|
engine/orchestration/src/main/java/com/cloud/agent/manager/AgentManagerImpl.java
Outdated
Show resolved
Hide resolved
server/src/main/java/com/cloud/resource/ResourceManagerImpl.java
Outdated
Show resolved
Hide resolved
@blueorangutan package |
@Pearl1594 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. |
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.
clgtm
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 12504 |
Description
Fixes: #10041
This PR provides Proper explanation for the global setting to avoid ambiguity
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?