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

Disconnect inventory on targeted refresh #16718

Merged
merged 1 commit into from
Jan 2, 2018

Conversation

borod108
Copy link

When doing a targeted refresh the condition to disconnect the target
from the ems was not granular enough.
For example in some cases when refreshing a template the 'folder' hashes
were not not empty while the 'template' hashes were, still the template
was not disconnected from the ems in that case.
The new condition is more accurate and should mitigate this kind of
scenarios.

The specs for this PR are here: ManageIQ/manageiq-providers-ovirt#184

@borod108
Copy link
Author

@agrare can you please take a look?
@miq-bot add_label gaprindashvili/yes

@borod108
Copy link
Author

while not directly fixing it, this is required to fix: https://bugzilla.redhat.com/show_bug.cgi?id=1511445

@borod108
Copy link
Author

@borod108 borod108 force-pushed the bugs/disconnect_ems_template branch from 07459d0 to 7f66cf9 Compare December 25, 2017 11:29
When doing a targeted refresh the condition to disconnect the target
from the ems was not granular enough.
For example in some cases when refreshing a template the 'folder' hashes
were not not empty while the 'template' hashes were, still the template
was not disconnected from the ems in that case.
The new condition is more accurate and should mitigate this kind of
scenarios.

This is part of prs that fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1511445
@borod108 borod108 force-pushed the bugs/disconnect_ems_template branch from 7f66cf9 to 5eb95a4 Compare December 25, 2017 11:32
@miq-bot
Copy link
Member

miq-bot commented Dec 25, 2017

Checked commit borod108@5eb95a4 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. ⭐

Copy link
Member

@agrare agrare left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@agrare agrare merged commit 14d1aa3 into ManageIQ:master Jan 2, 2018
@agrare agrare self-assigned this Jan 2, 2018
@agrare agrare added the bug label Jan 2, 2018
agrare added a commit to agrare/manageiq that referenced this pull request Jan 2, 2018
Fixes an issue found by ManageIQ#16718
where relationships are deleted even though disconnect is false.
agrare added a commit to agrare/manageiq that referenced this pull request Jan 2, 2018
Fixes an issue found by ManageIQ#16718
where relationships are deleted even though disconnect is false.
agrare added a commit to agrare/manageiq that referenced this pull request Jan 2, 2018
Fixes an issue found by ManageIQ#16718
where relationships are deleted even though disconnect is false.
@agrare agrare added this to the Sprint 77 Ending Jan 15, 2018 milestone Jan 2, 2018
agrare added a commit to agrare/manageiq that referenced this pull request Jan 3, 2018
Fixes an issue found by ManageIQ#16718
where relationships are deleted even though disconnect is false.
simaishi pushed a commit that referenced this pull request Jan 3, 2018
Disconnect inventory on targeted refresh
(cherry picked from commit 14d1aa3)
@simaishi
Copy link
Contributor

simaishi commented Jan 3, 2018

Gaprindashvili backport details:

$ git log -1
commit 17deb085b443493f0df96d8d807f1625fc13e5ca
Author: Adam Grare <[email protected]>
Date:   Tue Jan 2 09:19:36 2018 -0500

    Merge pull request #16718 from borod108/bugs/disconnect_ems_template
    
    Disconnect inventory on targeted refresh
    (cherry picked from commit 14d1aa3822d4f0f918a761e63a23683443037ab5)

agrare added a commit to agrare/manageiq that referenced this pull request Jan 3, 2018
Fixes an issue found by ManageIQ#16718
where relationships are deleted even though disconnect is false.
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