-
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
[Quota] Improve Quota balance calculation flow #8581
[Quota] Improve Quota balance calculation flow #8581
Conversation
@blueorangutan package |
@BryanMLima 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. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.19 #8581 +/- ##
============================================
- Coverage 14.96% 14.95% -0.01%
- Complexity 10995 11010 +15
============================================
Files 5373 5378 +5
Lines 469024 469802 +778
Branches 58818 57316 -1502
============================================
+ Hits 70197 70277 +80
- Misses 391056 391740 +684
- Partials 7771 7785 +14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8483 |
@JoaoJandre, do you think this PR can go to the |
@blueorangutan test |
@DaanHoogland a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
@BryanMLima sure |
[SF] Trillian test result (tid-9039)
|
@DaanHoogland, I don't think the added integration test ran. Does it need something more to run the integration tests in the plugin folder? |
@BryanMLima tests from |
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
@BryanMLima it passed. not sure if we should add more runners, but a runner for plugins in general would make sense. |
@DaanHoogland, the test should have not passed, as it threw an exception in the CI logs, I will take a look. |
@BryanMLima as you are still working on this, does it make sense to take it out of the milestone? cc @JoaoJandre |
@DaanHoogland, I will have a look, should be able to fix this PR, at max, late next week. |
@DaanHoogland, as you have more experience with the test matrix, did it ever happen in a scenario when the API had an incorrect signature, such as |
guess this time around it didn't fail? |
Nope, it still had the same error, even though the test is marked as “passed”. I removed the Quota plugin smoke test from the test matrix as I could not find the reason for the error message. The results from running the integration tests in my local lab is presented below for verification: runinfo
results
|
d2d5ec1
to
38f9ddf
Compare
@sureshanaparti, do you think this PR can go to the |
@BryanMLima , if @GutoVeronezi 's concerns are met, we can merge. |
@blueorangutan package |
@BryanMLima 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 9959 |
@blueorangutan test |
@DaanHoogland a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
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
[SF] Trillian test result (tid-10464)
|
Description
This PR aims to improve the overall Quota balance calculation by breaking into separate methods and calculating based on the period dates rather than the Quota usage records for a better understanding of its flow. This PR also introduces smoke test for Quota balance and normalized the aggregation usage calculation when it is not daily or hourly.
Types of changes
Feature/Enhancement Scale or Bug Severity
Bug Severity
How Has This Been Tested?
6
(i.e. volume).How did you try to break this feature and the system with this change?
I also tried to invert the steps in the test above
6
(i.e. volume).