Configuration Script Sources API #13626
Conversation
config/api.yml
Outdated
| @@ -42,6 +42,20 @@ | |||
| - :subcollection | |||
| :verbs: *g | |||
| :klass: Account | |||
| :ansible_repositories: | |||
| :description: Ansible Repositories | |||
There was a problem hiding this comment.
looking at the exposed class below, are these repositories or playbooks/scripts ? i.e. /api/ansible_scripts or /api/ansible_job_templates maybe ?
There was a problem hiding this comment.
/cc @bdunne for guidance/suggestions.
There was a problem hiding this comment.
I'm a little confused for a few reasons...
- Does it make sense for this endpoint Ansible specific?
- Do we usually look at only one subclass rather than the base class?
- The class doesn't match the endpoint / description
There was a problem hiding this comment.
@bdunne sorry for the confusion. Per the ticket assigned to me, it was my understanding that this was known as repositories. I should have reached out for more information.
Also based on that ticket and the specific use case of it, I thought it made sense to make an ansible specific endpoint.
cc: @h-kataria
config/api.yml
Outdated
| :collection_actions: | ||
| :get: | ||
| - :name: read | ||
| :identifier: configuration_script_view |
There was a problem hiding this comment.
Should this be configuration_script_source_view?
config/api.yml
Outdated
| :resource_actions: | ||
| :get: | ||
| - :name: read | ||
| :identifier: configuration_script_view |
| 'subcount' => 1, | ||
| 'name' => 'configuration_script_sources', | ||
| 'resources' => | ||
| [hash_including('href' => a_string_matching(configuration_script_sources_url(repository.id)))] |
There was a problem hiding this comment.
Was this intended to be on a new line?
db/fixtures/miq_product_features.yml
Outdated
| - :name: List | ||
| :description: List Configuration Script Sources | ||
| :feature_type: view | ||
| :identifier: configuration_script_sources_list |
There was a problem hiding this comment.
@h-kataria would appreciate your 👀 - should I wait for #13526 ? I am not sure it makes sense to have it ansible specific after reviewing that, would something like this work?
There was a problem hiding this comment.
@h-kataria please let us know when the move is done so @jntullo can update accordingly.
|
This pull request is not mergeable. Please rebase and repush. |
|
This pull request is not mergeable. Please rebase and repush. |
|
Checked commits jntullo/manageiq@fc6dfc6~...dc84f65 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
|
LGTM!! Thanks @jntullo |
| :collection_actions: | ||
| :get: | ||
| - :name: read | ||
| :identifier: embedded_configuration_script_source_view |
There was a problem hiding this comment.
Why not configuration_script_source_view? What is the embedded for?
There was a problem hiding this comment.
@bdunne There are two subclasses that UI can target: ExternalAutomationManager and EmbededAutomationManager. @lgalis and I discussed and felt that the External ones would be named without any decoration (e.g. configuration_script_source_view), but Embedded would be named with decoration (e.g. embedded_configuration_script_source_view)
There was a problem hiding this comment.
Although, I have to agree with @bdunne about the need for differentiating anything at the API level. A user should just get all of the configuration_script_sources they can see, regardless of what providers they are tied to. If they want ones from a particular provider, they would go through a subcollection.
I understand needing to filter the collection, but how do we do it with respect to something like VMs and Instances (i.e. Infra vs Cloud)
cc @abellotti
|
Yeah ,the more I read into this, the more wrong it seems. Just because the UI needs to differentiate between embedded vs external does not mean the API should at this level. At the top level, I'd expect all configuration_script_resources to be returned regardless of provider (but obivously only the ones I have permission to), and thus I don't understand why the identifier is limited to embedded. |
|
@jntullo please hold off on using this new endpoint until we discuss further. API/UI role identifier disconnect and general usage/filtering issues. Thanks. |
Add
GET /api/configuration_script_sourcesto the REST api. This is needed to populate the dropdown on the new "Playbook" Service Template Type screen.@miq-bot add_label enhancement, api
@miq-bot assign @abellotti
cc: @h-kataria
Links