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

Fixes VM extend retirement #62

Merged
merged 1 commit into from
Mar 17, 2017
Merged

Conversation

psachin
Copy link
Contributor

@psachin psachin commented Feb 28, 2017

The retires_on field was changed from a :date column to a :datetime which now requires the days (in seconds) be added to the time.

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

This issue was introduced in PR ManageIQ/manageiq#11156

@gmcculloug
Copy link
Member

@tinaafitz @mkanoor Please review.

cc @billfitzgerald0120

@gmcculloug
Copy link
Member

@tinaafitz Should this logic be changed to use the extend_retires_on method and is that method also broken now? app/models/mixins/retirement_mixin.rb#L54

@tinaafitz
Copy link
Member

@psachin Thanks for the fixing the methods. We added an extend_retires_on method as mentioned by @gmcculloug. I created a PR to address the days issue there.
PR is WIP because I have to fix a few tests:
ManageIQ/manageiq#14143

Once that PR gets merged, can you change the 2 Automate methods to use the new method.
vm.extend_retires_on(vm_retire_extend_days, vm.retires_on)

cc @mkanoor @billfitzgerald0120

@psachin
Copy link
Contributor Author

psachin commented Mar 3, 2017

@tinaafitz Sure

The retires_on field was changed from a `:date` column to a `:datetime`
which now requires the days(in seconds) be added to the time.

- Depends on: ManageIQ/manageiq#14143
- This issue was introduced in PR ManageIQ/manageiq#11156
- https://bugzilla.redhat.com/show_bug.cgi?id=1427503

Signed-off-by: Sachin <psachin@redhat.com>
@miq-bot
Copy link
Member

miq-bot commented Mar 3, 2017

Checked commit psachin@81535b3 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks good. 🍰

@gmcculloug
Copy link
Member

Merging to get fix in. @billfitzgerald0120 is working on refactoring and adding tests which will come in a followup PR.

@gmcculloug gmcculloug merged commit f8add0c into ManageIQ:master Mar 17, 2017
@gmcculloug gmcculloug added this to the Sprint 57 Ending Mar 27, 2017 milestone Mar 17, 2017
@psachin psachin deleted the BZ#1427503 branch March 18, 2017 04:58
@simaishi
Copy link
Contributor

Euwe backport details:

$ git log -1
commit 647760573e46e2807718b862d4dcdc78500a1881
Author: Greg McCullough <gmccullo@redhat.com>
Date:   Fri Mar 17 17:00:58 2017 -0400

    Merge pull request #62 from psachin/BZ#1427503
    
    Fixes VM extend retirement
    (cherry picked from commit f8add0c3bb362acf41f4607d790dbe0a2957e120)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1433980

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