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

Only generate vpor for the TimeProfile specified, not all TimeProfiles #13700

Merged
merged 2 commits into from
Jan 30, 2017

Conversation

Fryguy
Copy link
Member

@Fryguy Fryguy commented Jan 30, 2017

This fixes 2 bugs from #12792

  • Only generate vpor for the TimeProfile specified, not all TimeProfiles. Otherwise if you have 10 TimeProfiles, it would regenerate the default one 10 times, even though it isn't necessary
  • For backports, people will have records with a nil time_profile_id, and we should handle that. On master, I will make a data migration, so once I do that I can remove the hack.

@gtanzillo Please review
cc @NickLaMuro @dmetzger57

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

@miq-bot
Copy link
Member

miq-bot commented Jan 30, 2017

Checked commits Fryguy/manageiq@5a555db~...537af59 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
5 files checked, 0 offenses detected
Everything looks good. 🏆

@Fryguy Fryguy added bug and removed performance labels Jan 30, 2017
@gtanzillo gtanzillo added this to the Sprint 53 Ending Jan 30, 2017 milestone Jan 30, 2017
@gtanzillo gtanzillo merged commit 9d4f10d into ManageIQ:master Jan 30, 2017
simaishi pushed a commit that referenced this pull request Jan 31, 2017
Only generate vpor for the TimeProfile specified, not all TimeProfiles
(cherry picked from commit 9d4f10d)

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

Darga backport details:

$ git log -1
commit 6839e65af07eb92678536844bbdf7e6fa9ac8017
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Mon Jan 30 17:57:28 2017 -0500

    Merge pull request #13700 from Fryguy/vpor_rollup_performance
    
    Only generate vpor for the TimeProfile specified, not all TimeProfiles
    (cherry picked from commit 9d4f10d6e9c3f76cdb51675560558ea9c7b0c15f)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1417662

@Fryguy Fryguy deleted the vpor_rollup_performance branch January 31, 2017 14:53
@NickLaMuro
Copy link
Member

FYI @Fryguy, you are referencing the wrong PR in the description, should be #12972

Fryguy added a commit to Fryguy/manageiq that referenced this pull request Feb 8, 2017
After the backport of ManageIQ#12792 and ManageIQ#13700, a user can be in a state where
their VimPerformanceOperatingRanges are inconsistent.  Although it
doesn't really harm anything, this data migration makes it consistent
once again.
Fryguy added a commit to Fryguy/manageiq that referenced this pull request Mar 1, 2017
After the backport of ManageIQ#12792 and ManageIQ#13700, a user can be in a state where
their VimPerformanceOperatingRanges are inconsistent.  Although it
doesn't really harm anything, this data migration makes it consistent
once again.
simaishi pushed a commit that referenced this pull request Mar 2, 2017
Only generate vpor for the TimeProfile specified, not all TimeProfiles
(cherry picked from commit 9d4f10d)

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

simaishi commented Mar 2, 2017

Euwe backport details:

$ git log -1
commit bc2a7e3e4ea19c362ad5fbfd1efbd2f038ab89e0
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Mon Jan 30 17:57:28 2017 -0500

    Merge pull request #13700 from Fryguy/vpor_rollup_performance
    
    Only generate vpor for the TimeProfile specified, not all TimeProfiles
    (cherry picked from commit 9d4f10d6e9c3f76cdb51675560558ea9c7b0c15f)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1422647

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