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

[EUWE] Chargebacks for SCVMM (rollup-less) #13419

Merged
merged 73 commits into from
Jan 18, 2017

Conversation

isimluk
Copy link
Member

@isimluk isimluk commented Jan 10, 2017

This is equivalent of backporting the following prs and resolving conflicts.
#12049
#11925
#12616
#12610
#12608
#12792
#12914
#12670
#13221
#13229
#13269

Notes

@miq-bot add_label wip

There are three possible callers of build_results_for_report_chargeback
each of them sets @options to the same value that is later passed in as
an argument.

Let's have a single place to set it.

In future I want to get a rid of class variable. But it will take few
moments before we get there.

(cherry picked from commit 52a00de)
This was used for Ad-hoc chargeback schedule. It is unused since 2010
(a59b87cf535fd464d56d145bac12873cff65ea7d).

(cherry picked from commit 60554db)
This was never actual option. Only a design idea that never come to be
implemented. Let's drop this from documentation now, so we can
consolidate the options more.

(cherry picked from commit 2c83677)
This will allow us to encapsulate code manipulating it here.

(cherry picked from commit 2b7e9f4)
Timezone depends solely on Chargeback/ReportOptions.

(cherry picked from commit 7e72957)
The chargeback's timerange depends only on Chargeback report options.

(cherry picked from commit f5df793)
It is only dependent on ReportOptions and shall be part of it.

(cherry picked from commit edb6431)
(cherry picked from commit 473d14c)
Do instanciate early in the calculation process.

This will allows us to encapsulate code to these instances.

(cherry picked from commit 4ba9459)
It only manipulates Chargeback instance based on perf record.

(cherry picked from commit 6c980f0)
This lowers the indirection and complexity.

(cherry picked from commit e31d499)
As we see, it takes a very long name to describe what it does. Perhaps
we can lower the complexity in future.

(cherry picked from commit bc575d3)
As it better describes what it does. It calculates hourly cost for a
given metric.

(cherry picked from commit 16d00c3)
(cherry picked from commit b938855)
This will be needed once we will have ChargebackRateDetails with dynamic
rate/metric names.

(cherry picked from commit 9492963)
Not all the rates are relevant to the given chargeback. At this moment,
the field is calculated only if the given chargeback type is able to
carry on the result. In future, we may be able to calculate only those
fields/columns needed by user. Then this method will come in handy.

Using word fields vs. columns as it is common more common in the
chargeback's codebase now.

(cherry picked from commit 631a8ca)
(cherry picked from commit 4588c62)
This if branch is very rare. And we do not set `metric_value` in it,
that means it takes metric_value from the previous loop or crashes.

Introduced in f3506b7.

(cherry picked from commit bf417fc)
@miq-bot miq-bot added the wip label Jan 10, 2017
This will help us to dispart Chargeback calculations and rates cache.

(cherry picked from commit 07cd0ff)

Conflicts:
	app/models/chargeback.rb
	app/models/chargeback_container_image.rb
@miq-bot
Copy link
Member

miq-bot commented Jan 10, 2017

Checked commits isimluk/manageiq@fc542ae~...44aa15b with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
16 files checked, 10 offenses detected

app/models/chargeback/consumption_history.rb

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

app/models/chargeback/report_options.rb

  • ❕ - Line 34, Col 5 - Metrics/AbcSize - Assignment Branch Condition size for report_time_range is too high. [43.92/20]

app/models/chargeback_container_image.rb

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

app/models/chargeback_vm.rb

app/models/metric/chargeback_helper.rb

@isimluk isimluk changed the title [WIP] [EUWE] Chargebacks for SCVMM (rollup-less) [EUWE] Chargebacks for SCVMM (rollup-less) Jan 10, 2017
@isimluk
Copy link
Member Author

isimluk commented Jan 10, 2017

@miq-bot remove wip
@miq-bot add_label chargeback, enhancement

@miq-bot
Copy link
Member

miq-bot commented Jan 10, 2017

@isimluk unrecognized command 'remove', ignoring...

Accepted commands are: add_label, remove_label, rm_label, assign, set_milestone

@isimluk
Copy link
Member Author

isimluk commented Jan 10, 2017

@miq-bot remove_label wip

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.

Looks good. Thanks @isimluk 👍

@gtanzillo
Copy link
Member

@simaishi This one is good to merge.

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