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

target new refresh fails #16043

Merged
merged 1 commit into from
Sep 27, 2017
Merged

target new refresh fails #16043

merged 1 commit into from
Sep 27, 2017

Conversation

pkliczewski
Copy link
Contributor

When running publish vm workflow flow I saw:

[NoMethodError]: undefined method base_class' for NilClass:Class Method:[block in method_missing] relationship_mixin.rb:621:in block in remove_children'

and it is triggered from save_ems_inventory_no_disconnect. It means that link_invenotry do not
respects disconnect flag which is fixed by this patch.

@pkliczewski
Copy link
Contributor Author

@miq-bot assign @agrare

agrare
agrare reviewed Sep 26, 2017
View reviewed changes
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.

Great catch

app/models/ems_refresh/link_inventory.rb Outdated
@@ -26,7 +26,7 @@ def link_ems_inventory(ems, target, prev_relats, new_relats)
do_disconnect = target.kind_of?(ExtManagementSystem)

# Do the Folders to Folders relationships
update_relats(:folders_to_folders, prev_relats, new_relats) do |f|
update_relats(:folders_to_folders, prev_relats, new_relats, disconnect) do |f|
Copy link
Member

Choose a reason for hiding this comment

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

Could we just change this to set do_disconnect if disconnect is false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will check

@pkliczewski
target new refresh fails
d1ff546 
When running publish vm workflow flow I saw:

[NoMethodError]: undefined method `base_class' for NilClass:Class  Method:[block in method_missing]
relationship_mixin.rb:621:in `block in remove_children'

and it is triggered from `save_ems_inventory_no_disconnect`. It means that `link_invenotry` do not
respects disconnect flag which is fixed by this patch.
@miq-bot
Copy link
Member

miq-bot commented Sep 27, 2017

Checked commit pkliczewski@d1ff546 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

@masayag
Copy link
Contributor

masayag commented Sep 27, 2017 •
edited
Loading

Verified on 'publish vm' which used to fail prior to this patch on.
Before:

[----] E, [2017-09-26T20:24:02.457158 #20225:1131108] ERROR -- : MIQ(EmsRefresh.update_relats_by_ids) An error occurred while disconnecting id [494]: undefined method base_class' for NilClass:Class [----] E, [2017-09-26T20:24:02.457321 #20225:1131108] ERROR -- : [NoMethodError]: undefined method base_class' for NilClass:Class Method:[block in method_missing]

After:

[----] I, [2017-09-27T12:53:08.989551 #28186:8f910c] INFO -- : MIQ(EmsRefresh.link_ems_inventory) EMS: [ovirt-engine-master], id: [8] Linking EMS Inventory...Complete

agrare
agrare approved these changes Sep 27, 2017
View reviewed changes
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 great catch @pkliczewski

@agrare agrare added this to the Sprint 70 Ending Oct 2, 2017 milestone Sep 27, 2017
@agrare agrare merged commit d1ff546 into ManageIQ:master Sep 27, 2017
agrare added a commit that referenced this pull request Sep 27, 2017
@agrare
Merge pull request #16043 from pkliczewski/master
fa6d79e 
target new refresh fails
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@agrare agrare agrare approved these changes

Assignees

@agrare agrare

Labels
bug providers/inventory
Projects
None yet
Milestone
No milestone
Development

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