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.

Sign up for GitHub

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

Jump to bottom

Fix HostStorages being deleted by host targeted refresh #421

Merged
merged 2 commits into from
Jul 30, 2019
Merged

Fix HostStorages being deleted by host targeted refresh #421

merged 2 commits into from
Jul 30, 2019

Conversation

agrare
Copy link
Member

@agrare agrare commented Jul 29, 2019

A host targeted refresh was deleting related host_storages because the host_storages are actually refreshed by the storage target.

@agrare agrare force-pushed the fix_targeted_host_refresh branch 2 times, most recently from fcb729a to 89a852a Compare July 29, 2019 23:16
@agrare agrare added the bug label Jul 30, 2019
@agrare agrare force-pushed the fix_targeted_host_refresh branch from 89a852a to e488dd7 Compare July 30, 2019 00:07
Ladas
Ladas approved these changes Jul 30, 2019
View reviewed changes
Copy link
Contributor

@Ladas Ladas left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I see, the parent_inventory_collection was automatically inferred as :hosts (because the host_storages relations is :through => :hosts). Looks good. 👍

@Ladas
Copy link
Contributor

Ladas commented Jul 30, 2019

@agrare looks like the test is not passing though, the count is wrong

@agrare agrare force-pushed the fix_targeted_host_refresh branch from e488dd7 to 24f9942 Compare July 30, 2019 10:41
@miq-bot
Copy link
Member

miq-bot commented Jul 30, 2019

Checked commits agrare/manageiq-providers-vmware@55d201a~...24f9942 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. 🍪

@agrare
Copy link
Member Author

agrare commented Jul 30, 2019

Ah yeah I pushed a test for host_switches too soon, similar issue but dont' have a fix for that one yet

@agrare
Copy link
Member Author

agrare commented Jul 30, 2019

@Ladas there we go PTAL

Ladas
Ladas approved these changes Jul 30, 2019
View reviewed changes
Copy link
Contributor

@Ladas Ladas left a comment

Choose a reason for hiding this comment

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

👍 awesome

@Ladas Ladas merged commit 349b469 into ManageIQ:master Jul 30, 2019
@agrare agrare deleted the fix_targeted_host_refresh branch July 30, 2019 11:19
@Ladas Ladas added this to the Sprint 117 Ending Aug 5, 2019 milestone Jul 31, 2019
@agrare
Copy link
Member Author

agrare commented Jul 31, 2019

cc @simaishi

simaishi pushed a commit that referenced this pull request Jul 31, 2019
@Ladas @simaishi
Merge pull request #421 from agrare/fix_targeted_host_refresh
d938f4b 
Fix HostStorages being deleted by host targeted refresh

(cherry picked from commit 349b469)
@simaishi
Copy link
Contributor

Ivanchuk backport details:

$ git log -1
commit d938f4b858929d88dfcc6023ad3bc5233a985ab7
Author: Ladislav Smola <lsmola@redhat.com>
Date:   Tue Jul 30 13:14:13 2019 +0200

    Merge pull request #421 from agrare/fix_targeted_host_refresh
    
    Fix HostStorages being deleted by host targeted refresh
    
    (cherry picked from commit 349b469ed6d408a3d77b8df445357574f99e1754)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ladas Ladas Ladas approved these changes

Assignees

@Ladas Ladas

Labels
bug ivanchuk/backported
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@agrare @Ladas @miq-bot @simaishi