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

Use pluck on metric rollup query in chargeback #17560

Merged

Conversation

lpichler
Copy link
Contributor

@lpichler lpichler commented Jun 8, 2018

fixes #17592

Stop use AR collection in ConsumptionWithRollups.

Links [Optional]

https://bugzilla.redhat.com/show_bug.cgi?id=1566452

fifth part of fix

@lpichler lpichler changed the title Use pluck metric rollup query in chargeback [WIP] Use pluck on metric rollup query in chargeback Jun 8, 2018
@miq-bot miq-bot added the wip label Jun 8, 2018
@JPrause
Copy link
Member

JPrause commented Jun 11, 2018

@lpichler if this can be backported, please add the gaprindashvili/yes label

@JPrause
Copy link
Member

JPrause commented Jun 11, 2018

@miq-bot add_label blocker

@jrafanie
Copy link
Member

@lpichler This is failing tests... Can this be reviewed before #17538 is merged?

@miq-bot
Copy link
Member

miq-bot commented Jun 13, 2018

This pull request is not mergeable. Please rebase and repush.

@lpichler lpichler force-pushed the use_pluck_metric_rollup_query_in_chargeback branch 2 times, most recently from ad79e6f to 21cc480 Compare June 14, 2018 14:42
@JPrause
Copy link
Member

JPrause commented Jun 14, 2018

@lpichler where does this PR stand.

@miq-bot
Copy link
Member

miq-bot commented Jun 14, 2018

This pull request is not mergeable. Please rebase and repush.

…Field

GOAL: ConsumptionHistory is receiving array of values(got by pluck method)
instead of array MetricRollups

This commit
- this will be method used as argument in pluck method for base MetricRollup
query
- add @rollup_array to keep array of values which will be used in calculations
instead of array MetricRollups
- sort list of columns
…onsumptionWithRollups

- also add  self.col_index(column) as help method to access proper
  column from array of values
- resource_tag_names and all_tag_names don't removed yet from
Metric::ChargebackHelper because there is still usage in this helper
@lpichler lpichler force-pushed the use_pluck_metric_rollup_query_in_chargeback branch from 21cc480 to 541dfe7 Compare June 14, 2018 21:44
@lpichler lpichler changed the title [WIP] Use pluck on metric rollup query in chargeback Use pluck on metric rollup query in chargeback Jun 14, 2018
@miq-bot
Copy link
Member

miq-bot commented Jun 15, 2018

Checked commits lpichler/manageiq@9262bf5~...a4579d6 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
9 files checked, 0 offenses detected
Everything looks fine. 🏆

@lpichler lpichler mentioned this pull request Jun 15, 2018
6 tasks
@JPrause
Copy link
Member

JPrause commented Jun 15, 2018

@lpichler where does this PR stand. Will it be able to be merged today?

@lpichler
Copy link
Contributor Author

@JPrause yes, @gtanzillo is about to merge it.
@JPrause, @simaishi to keep order for backporting use issue #17592

@lpichler
Copy link
Contributor Author

@miq-bot add_label gaprindashvili/yes

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.

Nice work @lpichler! 👍

@gtanzillo gtanzillo added this to the Sprint 88 Ending Jun 18, 2018 milestone Jun 15, 2018
@gtanzillo gtanzillo merged commit 8180f01 into ManageIQ:master Jun 15, 2018
simaishi pushed a commit that referenced this pull request Jun 15, 2018
…_in_chargeback

Use pluck on  metric rollup query in chargeback
(cherry picked from commit 8180f01)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1591939
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit 4f909bc4a0c478bf4a08f4b9798b944f85ff65a8
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Fri Jun 15 13:54:53 2018 -0400

    Merge pull request #17560 from lpichler/use_pluck_metric_rollup_query_in_chargeback
    
    Use pluck on  metric rollup query in chargeback
    (cherry picked from commit 8180f01d945045b305f722aaaca6240aacd26961)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1591939

@lpichler lpichler deleted the use_pluck_metric_rollup_query_in_chargeback branch June 20, 2018 10:01
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.

Improve chargeback performance
6 participants