create configuration script sources #14006
Conversation
config/api.yml
Outdated
| @@ -537,12 +537,15 @@ | |||
| :description: Configuration Script Source | |||
| :options: | |||
| - :collection | |||
| :verbs: *g | |||
| :verbs: *gp | |||
There was a problem hiding this comment.
should probably be *gpppd so that you can also update and delete..
There was a problem hiding this comment.
@himdel will be added when we work on update and delete
There was a problem hiding this comment.
thanks, no pressure :) (except that UI work is waiting for this... :D)
jntullo
changed the title
|
@miq-bot remove_label wip |
| @@ -4,4 +4,10 @@ class ConfigurationScriptSource < ApplicationRecord | |||
| belongs_to :manager, :class_name => "ExtManagementSystem" | |||
|
|
|||
| virtual_total :total_payloads, :configuration_script_payloads | |||
|
|
|||
| def self.class_from_request_data(data) | |||
| manager = ExtManagementSystem.find(data['manager_resource']) | |||
There was a problem hiding this comment.
data['manager_resource'] is API payload dependent. Let's send in the object instead, i.e. class_for_manager(manager)
| parse_href(data['manager_resource']['href']).last | ||
| elsif data['manager_resource'].key?('id') | ||
| data['manager_resource']['id'] | ||
| end |
There was a problem hiding this comment.
if think this can simply be:
def validate_attrs(data)
raise 'must supply a manager resource' unless data['manager_resource']
end
| def create_resource(_type, _id, data) | ||
| attrs = validate_attrs(data) | ||
| type = ConfigurationScriptSource.class_from_request_data(attrs) | ||
| raise 'type not currently supported' unless type.respond_to?(:create_in_provider_queue) |
There was a problem hiding this comment.
with the change below, above can be something like:
validate_attrs(data)
manager_id = parse_id(data['manager_resource'], :providers)
manager = resource_search(manager_id, :providers, collection_class(:providers))
type = ConfigurationScriptSource.class_for_manager(manager)
...
There was a problem hiding this comment.
once the model supports other than ExtManagementSystem, we can provide an different variant of parse_id that supports a list of collections, where it returns the collection and id. This way we know for sure what href we are parsing (and the correct collection accessed in the following resource_search).
There was a problem hiding this comment.
as for the message, maybe something like "ConfigurationScriptSources cannot be added to #{manager_ident(manager)}"
|
This pull request is not mergeable. Please rebase and repush. |
| @@ -18,10 +18,30 @@ def delete_resource(type, id, _data = {}) | |||
| action_result(false, err.to_s) | |||
| end | |||
|
|
|||
| def create_resource(_type, _id, data) | |||
| validate_attrs(data) | |||
| manager_id = parse_id(data['manager_resource'], :providers) | |||
There was a problem hiding this comment.
Can you add a check here before we do a resource_search,
raise "Must specify a valid manager_resource href or id" unless manager_id
and add a test for that bad_request, maybe passing in a bogus /api/users/10 href. Thanks.
|
Checked commits jntullo/manageiq@4f30275~...f2e1e79 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
|
Thanks @jntullo for this last update. LGTM!! 👍 |
Create configuration script sources
cc: @h-kataria @ZitaNemeckova
@miq-bot add_label enhancement, api, wip
@miq-bot assign @abellotti
Depends on #14173