delete authentication in provider #14307
Conversation
jntullo
changed the title
|
@miq-bot remove_label wip |
| task_id = auth.delete_in_provider_queue | ||
| action_result(true, "Deleting Authentication with id #{id}", :task_id => task_id) | ||
| rescue => err | ||
| raise "Could not delete authentication - #{err}" |
There was a problem hiding this comment.
Let's also return action_result(false, ...) for rescue case, consistency otherwise our bulk actions won't play nice.
Also, maybe update message to be consistent with others, maybe "Deleting Authentication id:#{id}" see helper methods provider_ident(), vm_ident(), maybe have something similar for authentications.
| @@ -1,4 +1,11 @@ | |||
| module Api | |||
| class AuthenticationsController < BaseController | |||
| def delete_resource(type, id, _data = {}) | |||
| auth = resource_search(id, type, collection_class(:authentications)) | |||
| task_id = auth.delete_in_provider_queue | |||
There was a problem hiding this comment.
Is delete_in_provider_queue always available on Authentication resources ? I only see those defined in manageiq/providers/ansible_tower/shared/automation_manager/*
Should we do a respond_to? first then do the new logic otherwise the current (super) path ?
There was a problem hiding this comment.
should we support authentications subcollection delete ?
i.e. DELETE /api/configuration_script_payloads/:id/authentications/:a_id (and bulk equiv)
There was a problem hiding this comment.
@abellotti yeah, I wasn't sure if we should support DELETE. I guess it doesn't make sense because you don't get the task information back. I will remove it from the others as well
| private | ||
|
|
||
| def authentication_ident(auth) | ||
| "Authentication id:#{auth} name: '#{auth.name}'" |
There was a problem hiding this comment.
you probably meant id:#{auth.id}
config/api.yml
Outdated
| @@ -233,7 +233,7 @@ | |||
| :options: | |||
| - :collection | |||
| - :subcollection | |||
| :verbs: *gpd | |||
| :verbs: *gp | |||
There was a problem hiding this comment.
Why is the DELETE action removed on resource. You can add that back. We support DELETE on resources that are zapped async, i.e. /api/providers, if user sends a DELETE HTTP action, it's for fire and forget, they won't care about the task_id. If they care, they'd simply use the delete POST action and leverage our action result signature.
| @@ -1,4 +1,18 @@ | |||
| module Api | |||
| class AuthenticationsController < BaseController | |||
| def delete_resource(type, id, _data = {}) | |||
| auth = resource_search(id, type, collection_class(:authentications)) | |||
| raise 'type not currently supported' unless auth.respond_to?(:delete_in_provider_queue) | |||
There was a problem hiding this comment.
Maybe update the message. maybe something like "Delete not supported for #{authentication_ident(auth)}"
|
Checked commits jntullo/manageiq@aa8e2e9~...88ec961 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
|
Thanks @jntullo for the update and Enhancement. LGTM!! 😍 |
Update delete of authentications (whoops how'd that get in there) to use
delete_in_provider_queuein #14305@miq-bot add_label enhancement, api, wip
@miq-bot assign @abellotti