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.

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

Chargeback no longer depend on MetricRollup #13221

Merged
merged 13 commits into from
Dec 19, 2016

Conversation

isimluk
Copy link
Member

@isimluk isimluk commented Dec 16, 2016

What

Extract MetricRollup manipulation into Consumption and ConsumptionHistory classes.

Why

Needed for upcoming chargebacks without metric rollups.

Details

Previously, chargeback relied on existing metric_rollups and chargeback code was tightly bound to metric_rollup class. In future, we want to charge for VMs that do not have metric_rollups assigned.

Thus, we need to break the bound between chargeback code and metric_rollups. We need chargeback code to depend only on Consumption class fetching this class from ConsumptionHistory. Then we can change Consumption to operate over VMs without metric_rollups.

/cc @lpichler -- Thanks for consultation, sir!

@miq-bot add_label chargeback, refactoring, euwe/no
@miq-bot assign @gtanzillo

@miq-bot
Copy link
Member

miq-bot commented Dec 16, 2016

Checked commits isimluk/manageiq@b234e2a~...0ac994e with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
8 files checked, 2 offenses detected

app/models/chargeback/consumption_history.rb

  • ❕ - Line 7, Col 5 - Metrics/AbcSize - Assignment Branch Condition size for for_report is too high. [21.63/20]

app/models/chargeback_container_image.rb

  • ❕ - Line 90, Col 3 - Metrics/AbcSize - Assignment Branch Condition size for init_extra_fields is too high. [22.23/20]

@isimluk isimluk changed the title Extract class :: ConsumptionHistory Chargeback no longer depend on MetricRollup Dec 16, 2016
Copy link
Member

@gtanzillo gtanzillo left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@gtanzillo gtanzillo added this to the Sprint 51 Ending Jan 2, 2017 milestone Dec 19, 2016
@gtanzillo gtanzillo merged commit 9d47d0a into ManageIQ:master Dec 19, 2016
@simaishi
Copy link
Contributor

Backported to Euwe via #13419

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants