Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.18 #8030 +/- ##
============================================
+ Coverage 13.16% 13.17% +0.01%
- Complexity 9199 9207 +8
============================================
Files 2724 2724
Lines 258109 258149 +40
Branches 40228 40235 +7
============================================
+ Hits 33970 34003 +33
Misses 219837 219837
- Partials 4302 4309 +7 ☔ View full report in Codecov by Sentry. |
|
@blueorangutan package |
|
|
||
| @Override | ||
| public void execute() { | ||
| CallContext.current().setEventDetails(String.format("Tariff: %s, description: %s, value: %s", getName(), getDescription(), getValue())); |
There was a problem hiding this comment.
IMO, It would be better to override getApiResourceType() & getApiResourceId() methods. This would also link the events with the quota resources.
@harikrishna-patnala @DaanHoogland What do you think?
There was a problem hiding this comment.
Do you mean to add the fields resource_type and resource_id to the quota_tariffs table and set and return those values?
As tarriffs can change and need to keep historic data, I am not sure, but it might work. @GutoVeronezi can you chime in?
There was a problem hiding this comment.
No. I meant for Quota resource itself. Here is the javadoc for getApiResourceId() method.
This will require adding a key ApiCommandResourceType.java as well.
/**
* Commands that generate action events associated to a resource and
* async commands that want to be tracked as part of the listXXX commands
* need to provide implementations of the two following methods,
* getApiResourceId() and getApiResourceType()
*
* getApiResourceId() should return the id of the object the async command is executing on
* getApiResourceType() should return a type from the ApiCommandResourceType enumeration
*/As per my understanding, this will add a link in UI for events view. And allows to filter events per resource type & resource id which is quota in this case.
There was a problem hiding this comment.
I made the changes to allow users filter quota tariff events in UI. Could you guys take a look?
|
@blueorangutan package |
|
@DaanHoogland a [SF] 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]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 7292 |
|
@blueorangutan test |
|
@DaanHoogland a [SF] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
hsato03
force-pushed
the
quota-tariff-events
branch
from
bc6fbe0 to
d8ed21b
Compare
|
@blueorangutan package |
|
@vishesh92 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]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8245 |
|
@blueorangutan package |
|
@hsato03 are we targeting 20 with this PR (cc @JoaoJandre )? |
|
@DaanHoogland @JoaoJandre Maybe we can target it to the 4.18.2. What do you think? |
|
@blueorangutan package |
|
@vishesh92 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]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8555 |
|
@blueorangutan test |
|
@blueorangutan package |
|
@DaanHoogland 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]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8632 |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-9189)
|
|
@hsato03 I think this needs testing but as you guys are the only heavy users of quota, can you find one of your colleagues to verify this, please? |
GutoVeronezi
left a comment
GutoVeronezi
left a comment
There was a problem hiding this comment.
CLGTM
I'll try to test it in the next days.
|
@hsato03, the events are being created correctly and the filter in the UI is working. However, the
|
Thanks. I addressed this in the last commit. Could you take a look again?
|
|
@blueorangutan package |
|
@GutoVeronezi 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. |
|
I think one more round of tests is needed (just for sanity); aside that, I think this one is good to go. |
|
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8838 |
|
@blueorangutan test |
|
@DaanHoogland a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
[SF] Trillian test result (tid-9378)
|
Co-authored-by: Henrique Sato <henrique.sato@scclouds.com.br>
7 participants
Description
While creating, updating or removing a quota tariff there are no events for any of these actions.
To address that, every time a quota tariff is created, updated or removed, a new event is created containing some details of these operations.
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
I created, updated and removed a quota tariff. Then I accessed the
Eventstab and all the operations were there.