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

Add resource relation to MiqSchedule #17581

Merged
merged 3 commits into from
Jun 15, 2018

Conversation


_log.info("Queueing start of schedule id: [#{id}] [#{sched.name}] [#{sched.towhat}] [#{method}]...complete")
msg
else
_log.warn("[#{sched.name}] no such action: [#{method}], aborting schedule")
return
Copy link
Member

Choose a reason for hiding this comment

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

This return is redundant now, right?

@@ -39,6 +42,11 @@ class MiqSchedule < ApplicationRecord
default_value_for :enabled, true
default_value_for(:zone_id) { MiqServer.my_server.zone_id }

def resource
# HACK: this should be a real relation, but for now it's using a reserve_attribute for backport reasons
Object.const_get(towhat).find_by(:id => resource_id)
Copy link
Member

@jrafanie jrafanie Jun 14, 2018

Choose a reason for hiding this comment

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

Are we deprecating towhat here? The commit says we are but maybe it's coming in a future PR?

Copy link
Member

@jrafanie jrafanie left a comment

Choose a reason for hiding this comment

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

@bdunne is there a BZ for this to eventually be backported?

@Fryguy @kbrock can you review the reserves usage?

@bdunne
Copy link
Member Author

bdunne commented Jun 14, 2018

@jrafanie Thanks, I cleaned up the return. There is no BZ since it's a RFE that needs to be backported.

@miq-bot
Copy link
Member

miq-bot commented Jun 14, 2018

Checked commits bdunne/manageiq@e6991fd~...4c3a53f with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🏆

def resource
# HACK: this should be a real relation, but for now it's using a reserve_attribute for backport reasons
return unless resource_id
towhat.safe_constantize.find_by(:id => resource_id)
Copy link
Member

Choose a reason for hiding this comment

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

I can't unsee 'tow hat' now, thanks @bdunne

@jrafanie
Copy link
Member

LGTM, I'll give others a little bit to verify the reserves usage since I haven't seen or used it in a while.

@gtanzillo gtanzillo self-assigned this Jun 15, 2018
@gtanzillo gtanzillo added this to the Sprint 88 Ending Jun 18, 2018 milestone Jun 15, 2018
@gtanzillo gtanzillo merged commit 381f94a into ManageIQ:master Jun 15, 2018
@bdunne bdunne deleted the miq_schedule_resources branch June 15, 2018 17:50
simaishi pushed a commit that referenced this pull request Jul 25, 2018
Add resource relation to MiqSchedule
(cherry picked from commit 381f94a)
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit 5b709f4f9211ae6cb1c0181141a17b29ec3b359c
Author: Gregg Tanzillo <gtanzill@redhat.com>
Date:   Fri Jun 15 13:44:03 2018 -0400

    Merge pull request #17581 from bdunne/miq_schedule_resources
    
    Add resource relation to MiqSchedule
    (cherry picked from commit 381f94aa73db00c66f7fa51f253e303cbb2168f6)

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