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

Utility function to update PxeMenu list #19134

Merged
merged 1 commit into from
Oct 17, 2019

Conversation

miha-plesko
Copy link
Contributor

With this commit we introduce a utility method on PxeServer model to handle PxeMenus list update. Until now, miq-api was always recreating all menus upon server update, but often we can actually just preserve exisitng menus. This method calculates what menus to add, what to preserve and what to remove, and then executes it.

Depends on: #19126
Fixes #18940

@miq-bot add_label bug,ivanchuk/yes
@miq-bot assign @agrare

@agrare
Copy link
Member

agrare commented Aug 12, 2019

I'm not familiar with the pxe server or menus @gmcculloug do you know?

@miha-plesko
Copy link
Contributor Author

I think all PXE stuff was marked for deletion already because no one was using it. Until we needed it for Redfish. In other words, I don't think anyone is very familiar since 2015 or so when it was last changed. On the bright side, that means we don't have to worry much that we'd break anything for other users, because there are none :)

@gmcculloug gmcculloug requested a review from bdunne August 12, 2019 14:46
@Fryguy
Copy link
Member

Fryguy commented Aug 12, 2019

@bdunne Please review.
@miq-bot assign @bdunne

@miq-bot miq-bot assigned bdunne and unassigned agrare Aug 12, 2019
app/models/pxe_server.rb Outdated Show resolved Hide resolved
app/models/pxe_server.rb Outdated Show resolved Hide resolved
With this commit we introduce a utility method on PxeServer model
to handle PxeMenus list update. Until now, miq-api was always
recreating all menus upon server update, but often we can actually
just preserve exisitng menus. This method calculates what menus to
add, what to preserve and what to remove, and then executes it.

Fixes ManageIQ#18940

Signed-off-by: Miha Pleško <[email protected]>
Copy link
Contributor Author

@miha-plesko miha-plesko left a comment

Choose a reason for hiding this comment

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

Thanks for review @Fryguy @bdunne I've applied your suggestions.

app/models/pxe_server.rb Outdated Show resolved Hide resolved
app/models/pxe_server.rb Outdated Show resolved Hide resolved
spec/models/pxe_server_spec.rb Show resolved Hide resolved
@miq-bot
Copy link
Member

miq-bot commented Aug 13, 2019

Checked commit xlab-si@9e1366a with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 👍

@martinpovolny
Copy link
Member

@bdunne : Any reason not to merge this? (I am doing a cleanup of the opened PRs in the API and this is prerequisite for one.)

@bdunne bdunne merged commit b6fd844 into ManageIQ:master Oct 17, 2019
@bdunne bdunne added this to the Sprint 123 Ending Oct 28, 2019 milestone Oct 17, 2019
@bdunne bdunne added the core label Oct 17, 2019
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.

PXE Images refresh duplicates entries
6 participants