[WIP] Rate selection using union of all tags in reporting period #15857
Conversation
|
Checked commit gtanzillo@3520af6 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 app/models/chargeback/consumption_with_rollups.rb
|
|
@gtanzillo failing test is about determination from multiple cases as I did, but the test is using one metric rollup and changing tag_names in the metric_rollup(this cannot happen in production) but this PR is using memoizing of tag_names(all memoizing here is ok and I think it helps with performance) and we need to refresh it in spec for example by this way: diff --git a/spec/models/chargeback_vm_spec.rb b/spec/models/chargeback_vm_spec.rb
index f284a6e0e6..fce21a5310 100644
--- a/spec/models/chargeback_vm_spec.rb
+++ b/spec/models/chargeback_vm_spec.rb
@@ -480,6 +480,10 @@ describe ChargebackVm do
it "return only one chargeback rate according to tag name of Vm" do
[rate_assignment_options_1, rate_assignment_options_2].each do |rate_assignment|
metric_rollup.tag_names = rate_assignment[:tag].first.tag.send(:name_path)
+ # as we are using one metric_rollup and changing it to get then we need to refresh
+ # instance variables of consumption
+ consumption.instance_variable_set(:@tag_names, nil)
+ consumption.instance_variable_set(:@hash_features_affecting_rate, nil)
uniq_rates = chargeback_vm.get(consumption)
expect([rate_assignment[:cb_rate]]).to match_array(uniq_rates)
end |
| @@ -10,7 +10,7 @@ def initialize(metric_rollup_records, start_time, end_time) | |||
| end | |||
|
|
|||
| def tag_names | |||
| first_metric_rollup_record.tag_names.split('|') | |||
| @tag_names ||= @rollups.map { |m| m.tag_names.split('|') }.flatten.uniq | |||
There was a problem hiding this comment.
I think that some tag_names on the rollup can be nil, so probably we need something like:
@rollups.where.not(:tag_names => nil ).map { |m| m.tag_names.split('|') }.flatten.uniq
Previously, tag list was determined from first metric rollup of the period but it is not enough to use tag list only from first rollup and tag list can be changed during period. Now we are using tag list from metric rollup records of consumption period. Source: ManageIQ#15857
|
Closing because this is replaced by #15888 |
|
This pull request is not mergeable. Please rebase and repush. |
No description provided.