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 #3971 - warning shown when PXE template is edited and has hosts in build mode #2501

Closed
wants to merge 1 commit into from

Conversation

xprazak2
Copy link
Contributor

Warning shown when PXE template is edited and has hosts in build mode

@domcleal domcleal changed the title Fixes #3971 Fixes #3971 - warning shown when PXE template is edited and has hosts in build mode Jun 30, 2015
@domcleal
Copy link
Contributor

My preference would be to add a feature to rewrite the templates, either automatically or otherwise, rather than put the onus entirely on the user. Any thoughts about that? We have http://projects.theforeman.org/issues/2267 and related tracking this.

@@ -67,6 +67,11 @@ def permitted_actions(template)
end
end

def pxe_with_building_hosts?(template)
kinds = ["PXELinux", "PXEGrub", "iPXE"]
template.operatingsystems.map(&:hosts).flatten.any?(&:build) && !template.template_kind.nil? && kinds.include?(template.template_kind.name)
Copy link
Contributor

Choose a reason for hiding this comment

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

We should check the performance of this host lookup query to check it's not performing a huge number of SQL queries. We should probably also use where(:build => true) to filter in the database.

@xprazak2
Copy link
Contributor Author

xprazak2 commented Jul 1, 2015

I made changes according to comments, I also added a feature that can rewrite the templates:
#2508

def pxe_with_building_hosts?(template)
kinds = ["PXELinux", "PXEGrub"]
os_ids = template.operatingsystem_ids
!template.template_kind.nil? && kinds.include?(template.template_kind.name) && Host.where(:build => true, :operatingsystem_id => os_ids).count > 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that using template_kind is triggering test failures as it only exists in ProvisioningTemplate, not Ptable.

@lzap
Copy link
Member

lzap commented Jul 2, 2015

I don't see any code that triggers regeneration. Have you pushed properly? Also, tests failed.

@domcleal
Copy link
Contributor

domcleal commented Jul 2, 2015

See the linked PR, #2508.

kinds = ["PXELinux", "PXEGrub"]
os_ids = template.operatingsystem_ids
template.respond_to?(:template_kind) &&
!template.template_kind.nil? &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick, template.template_kind.present? rather than the negation, or even combine it with the line above and do template.try(:template_kind).present? which'll work when it doesn't respond to.

@domcleal
Copy link
Contributor

domcleal commented Jul 8, 2015

[test]

@domcleal
Copy link
Contributor

domcleal commented Jul 9, 2015

Merged as a377f77, thanks @xprazak2.

@domcleal domcleal closed this Jul 9, 2015
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.

4 participants