-
Notifications
You must be signed in to change notification settings - Fork 94
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
Include provider in OCP/cloud tag sql params #5261
Conversation
c937692
to
db1aae3
Compare
db1aae3
to
88a972f
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5261 +/- ##
=======================================
- Coverage 94.1% 94.1% -0.0%
=======================================
Files 375 375
Lines 31429 31429
Branches 4627 4627
=======================================
- Hits 29573 29571 -2
- Misses 1185 1187 +2
Partials 671 671 |
For each ocp on cloud row, there are two bill ids associated with it.
The question I have is why are we not filtering by the report_period_id within the tag summary sql? We could pass in the current_report_period_id to the tag_information call, because the line right before it utilizes it. Also, I am not sure if the title of this issue correctly describes all the changes being made in the PR. We are changing the tag sql params for all ocp on cloud providers. |
Thats a reasonable suggestion that probably reduces the dataset down even more than just by cluster. 😄 Long as this SQL still works as expected with the smaller dataset I'm fine with that approach. I'll push that change up and we can give it a whirl. Thanks Cody! |
81b1ccd
to
145281c
Compare
c552e22
to
e0dc473
Compare
a9c72f3
to
befd9d2
Compare
/retest |
Jira Ticket
COST-####
Description
This change will add source_uuid to reporting_ocp{cloud}tags_summary.sql in order to reduce the select scale set. The more clusters/providers a customer has the slower this query is going to become.
Additionally fixed up some logging.
Testing
Release Notes