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

[WIP] Remove cpu total from chargeback report #12829

Closed
wants to merge 2 commits into from

Conversation

lpichler
Copy link
Contributor

@lpichler lpichler commented Nov 24, 2016

because there are added MHz s and counts of cpu cores together.
CPU total cost make sense.

I introduced here b31330a exluding of columns from reports but we can revert it a remove by migration if future.

@miq-bot add_label chargeback, reporting, bug

@miq-bot assign @gtanzillo

@@ -223,4 +225,20 @@ def remove_loading_relations_for_virtual_custom_attributes
vc_attributes = CustomAttributeMixin.select_virtual_custom_attributes(cols).present?
include.delete(:custom_attributes) if vc_attributes.present? && include && include[:custom_attributes].blank?
end

Copy link
Member

Choose a reason for hiding this comment

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

@lpichler Please add a comment here indicating that these methods are for supporting legacy reports that may have some of the columns that are being removed by this PR in them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

comment added, but I moved excluding columns to the after_initialize,
because before array operation .push does not work as I expected.

a = MiqReport.last
a.cols.push("aaa")

@lpichler lpichler closed this Dec 2, 2016
@lpichler lpichler reopened this Dec 2, 2016
@lpichler lpichler force-pushed the remove_total_cpu branch 2 times, most recently from ac848ad to 1a2f993 Compare December 2, 2016 12:18
@miq-bot
Copy link
Member

miq-bot commented Dec 13, 2016

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

@miq-bot
Copy link
Member

miq-bot commented Dec 14, 2016

Checked commits lpichler/manageiq@7b3b3ea~...5e68ceb with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
2 files checked, 0 offenses detected
Everything looks good. ⭐

@lpichler lpichler changed the title Remove cpu total from chargeback report [WIP] Remove cpu total from chargeback report Dec 20, 2016
@chessbyte chessbyte added the wip label Jan 3, 2017
@lpichler
Copy link
Contributor Author

closing , replaced by @isimluk's PRs: #15261 and #15260

@lpichler lpichler closed this Jun 23, 2017
@lpichler lpichler deleted the remove_total_cpu branch June 23, 2017 16:06
@isimluk
Copy link
Member

isimluk commented Jun 23, 2017

Oh, I thought I have seen that idea before. Sorry for the hijack bro. 🍔

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.

None yet

5 participants