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

Use destroy_queue for provider delete #217

Merged
merged 1 commit into from
Nov 20, 2017
Merged

Use destroy_queue for provider delete #217

merged 1 commit into from
Nov 20, 2017

Conversation

jntullo
Copy link

@jntullo jntullo commented Nov 20, 2017

Changes to ExtManagementSystem now require the use of schedule_destroy_queue to queue the deletion of the provider. Without this change, provider deletion is silently failing due to a before_hook on destroy that checks if there are any active queues for the provider. The orchestrate_delete method that will now be called will resolve this.

https://bugzilla.redhat.com/show_bug.cgi?id=1501941

@miq-bot add_label bug, gaprindashvili/yes, blocker

cc: @gtanzillo @zeari

@zeari
Copy link

zeari commented Nov 20, 2017

cc @durandom

raise BadRequestError, "Must specify an id for deleting a #{type} resource" unless id
provider = resource_search(id, type, collection_class(type))
task = provider.class.schedule_destroy_queue(provider.id)
action_result(true, "#{provider_ident(provider)} deleting", :task_id => task.id)
Copy link

Choose a reason for hiding this comment

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

I think you should use destroy_queue instead of schedule_destroy_queue because it also schedules the destroy of child managers

@jntullo jntullo changed the title Use schedule_destroy_queue for provider delete Use destroy_queue for provider delete Nov 20, 2017
@jntullo
Copy link
Author

jntullo commented Nov 20, 2017

@miq-bot assign @abellotti

Changes to ExtManagementSystem now require the use of destroy_queue to queue the deletion of the provider. Without this change, provider deletion is silently failing due to a before_hook on destroy that checks if there are any active queues for the provider. The orchestrate_delete method that will now be called will resolve this.

https://bugzilla.redhat.com/show_bug.cgi?id=1501941
@miq-bot
Copy link
Member

miq-bot commented Nov 20, 2017

Checked commit jntullo@e91c02a 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. 🏆

@Fryguy Fryguy merged commit 4d00d26 into ManageIQ:master Nov 20, 2017
@Fryguy Fryguy added this to the Sprint 74 Ending Nov 27, 2017 milestone Nov 20, 2017
raise BadRequestError, "Must specify an id for deleting a #{type} resource" unless id
provider = resource_search(id, type, collection_class(type))
task = provider.destroy_queue
action_result(true, "#{provider_ident(provider)} deleting", :task_id => task.id, :parent_id => id)
Copy link
Member

@abellotti abellotti Nov 20, 2017

Choose a reason for hiding this comment

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

why the parent_id here ? that's different behavior than the old destroy_provider method.

Copy link
Author

Choose a reason for hiding this comment

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

@abellotti yeah, I'm confused as to how it was returning the href, because unless the parent_id is specified, it wasn't.

simaishi pushed a commit that referenced this pull request Nov 20, 2017
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit 178e410f9574a650a5396b1f56cd728a38771997
Author: Jason Frey <fryguy9@gmail.com>
Date:   Mon Nov 20 13:52:22 2017 -0500

    Merge pull request #217 from jntullo/bz/provider_orchestrate_delete
    
    Use destroy_queue for provider delete
    (cherry picked from commit 4d00d2677c9e894b0a7f04604b69913338145241)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1515449

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.

6 participants