Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[COST-3554] Add column to store amortized cost with markup #4378

Merged
merged 9 commits into from
May 31, 2023
Merged

[COST-3554] Add column to store amortized cost with markup #4378

merged 9 commits into from
May 31, 2023

Conversation

samdoran
Copy link
Contributor

@samdoran samdoran commented May 18, 2023 •
edited

Jira Ticket

COST-3554

Description

Customers can add a markup value. We need to store the calculated amortized cost with markup.

@samdoran samdoran requested review from a team as code owners May 18, 2023 21:42
@samdoran samdoran mentioned this pull request May 18, 2023
Merged
1 task
@samdoran samdoran added the smoke-tests pr_check will build the image and run minimal required smokes label May 18, 2023
@samdoran
Copy link
Contributor Author

/retest

@samdoran samdoran force-pushed the COST-3554/markup-cost-calculated branch from e6fac69 to a0d3687 Compare May 22, 2023 16:26
@codecov
Copy link

codecov bot commented May 22, 2023 •
edited

Codecov Report

Merging #4378 (bddc776) into main (efc0007) will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@          Coverage Diff          @@
##            main   #4378   +/-   ##
=====================================
  Coverage   95.9%   95.9%           
=====================================
  Files        695     696    +1     
  Lines      71670   71696   +26     
  Branches    5734    5734           
=====================================
+ Hits       68699   68727   +28     
  Misses      1988    1988           
+ Partials     983     981    -2

@samdoran
Copy link
Contributor Author

/retest

@samdoran samdoran force-pushed the COST-3554/markup-cost-calculated branch 2 times, most recently from df62554 to eb83f0a Compare May 23, 2023 17:09
@samdoran samdoran added the django migration Change requires a Django database migration label May 23, 2023
@samdoran
Copy link
Contributor Author

/retest

@samdoran samdoran marked this pull request as draft May 25, 2023 15:32
@samdoran samdoran force-pushed the COST-3554/markup-cost-calculated branch from eb83f0a to 92de42f Compare May 25, 2023 18:00
@samdoran
Copy link
Contributor Author

I discovered we are missing calculated_amortized_cost for AWS filtered by OpenShift. I'll have to update those SQL files as well.

@samdoran samdoran force-pushed the COST-3554/markup-cost-calculated branch from 7d99236 to 96cf1c0 Compare May 26, 2023 13:25
@samdoran samdoran marked this pull request as ready for review May 26, 2023 13:25
@samdoran
Copy link
Contributor Author

/retest

lcouzens
lcouzens reviewed May 26, 2023
View reviewed changes
Copy link
Contributor

@lcouzens lcouzens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one little change and I think we are good

koku/masu/database/trino_sql/reporting_ocpawscostlineitem_daily_summary.sql Outdated Show resolved Hide resolved
@samdoran samdoran force-pushed the COST-3554/markup-cost-calculated branch from 96cf1c0 to b030f96 Compare May 26, 2023 16:22
@samdoran
Copy link
Contributor Author

/retest

@samdoran samdoran force-pushed the COST-3554/markup-cost-calculated branch from 6962572 to 5113c2d Compare May 26, 2023 20:58
cgoodfred
cgoodfred requested changes May 30, 2023
View reviewed changes
Copy link
Contributor

@cgoodfred cgoodfred left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think were almost there, just a few things related to those latest updates and we should be good. Thanks for slogging through all these changes.

scripts/cji_scripts/migrate_trino.py Outdated Show resolved Hide resolved
koku/reporting/provider/aws/openshift/models.py Show resolved Hide resolved
@samdoran samdoran force-pushed the COST-3554/markup-cost-calculated branch from 5113c2d to 1ec551a Compare May 30, 2023 18:48
cgoodfred
cgoodfred approved these changes May 31, 2023
View reviewed changes
Copy link
Contributor

@cgoodfred cgoodfred left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for all the updates Sam!

@samdoran
Copy link
Contributor Author

/retest

@sonarcloud
Copy link

sonarcloud bot commented May 31, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

93.3% 93.3% Coverage
0.0% 0.0% Duplication

@samdoran samdoran added the trino migration Change requires a Trino migration label May 31, 2023
@samdoran
Copy link
Contributor Author

/retest

3 similar comments
@samdoran
Copy link
Contributor Author

/retest

@samdoran
Copy link
Contributor Author

/retest

@samdoran
Copy link
Contributor Author

/retest

@samdoran samdoran merged commit 67c284f into main May 31, 2023
13 checks passed
@samdoran samdoran deleted the COST-3554/markup-cost-calculated branch May 31, 2023 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lcouzens lcouzens lcouzens left review comments

@cgoodfred cgoodfred cgoodfred approved these changes

Assignees
No one assigned
Labels
django migration Change requires a Django database migration smoke-tests pr_check will build the image and run minimal required smokes smokes-required trino migration Change requires a Trino migration
Projects
None yet
Milestone
No milestone
3 participants
@samdoran @lcouzens @cgoodfred