Ansible refresh inventory #13807
Ansible refresh inventory #13807
Conversation
durandom
force-pushed
the
ansible_refresh_inventory
branch
from
8f88ac6
to
aa522c1
Compare
|
This pull request is not mergeable. Please rebase and repush. |
durandom
force-pushed
the
ansible_refresh_inventory
branch
from
aa522c1
to
28b34b1
Compare
durandom
force-pushed
the
ansible_refresh_inventory
branch
from
28b34b1
to
6d28a00
Compare
|
Some comments on commits durandom/manageiq@abba7cb~...6d28a00 spec/models/manageiq/providers/ansible_tower/automation_manager/refresher_spec.rb
|
|
Checked commits durandom/manageiq@abba7cb~...6d28a00 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
| # FIXME: its not really nice how I have to build a manager_ref / uuid here | ||
| p = target.playbooks.find_or_build("#{i.id}__#{playbook_name}") | ||
| # FIXME: how about just adding `o` - configuration_script_source here? | ||
| p[:configuration_script_source] = target.configuration_script_sources.lazy_find(i.id.to_s) |
There was a problem hiding this comment.
if it's always 'o', you can just put it there :-)
|
|
||
| i.playbooks.each do |playbook_name| | ||
| # FIXME: its not really nice how I have to build a manager_ref / uuid here | ||
| p = target.playbooks.find_or_build("#{i.id}__#{playbook_name}") |
There was a problem hiding this comment.
so the manager_ref is "#{i.id}__#{playbook_name}" ?
it might be better not to change manager_ref, since you will need to parse it when talking to provider? E.g. doing some operation with playbook.
There was a problem hiding this comment.
Well, the playbook is just a name. So to make it really uniq, I need to scope it under project_id
GET /api/v1/projects/4/playbooks/
HTTP 200 OK
Allow: GET, HEAD, OPTIONS
Content-Type: application/json
Vary: Accept
X-API-Time: 0.039s
[
"hello_world.yml"
]
There was a problem hiding this comment.
right, so it's better to have in unchanged in our db, so e.g. it you want to delete a playbook you know you need playbook.manager_ref and playbook.configuration_script_source.manager_ref
So you don't need to do a playbook.manager_ref.split("__")
I was proposing another key for pure unique check in #13806
There was a problem hiding this comment.
so it's better to have in unchanged in our db
what should I set it to then? If I do this
target.playbooks.find_or_build(playbook_name)
the tests fail, because there are duplicate playbook names across projects
There was a problem hiding this comment.
So this case would need find_or_build to take in more than 1 attribute. Are there other use cases that need this too?
There was a problem hiding this comment.
Yeah, we need to add the find_or_build_by, as we talked in the other PR
In AWS, I have several entities with unique key.size >= 2
|
Overall looks good to me. @Ladas is also reviewing |
|
@blomquisg for me it's just nits, so we can merge and tweak those later. :-) @durandom has nice FIXMEs for those :-) I think the refresh code looks really nice now. :-) @durandom is the King of the APIs :-) |
|
@blomquisg if you merge this, you can call me whatever you want :) I'll fix the FIXMEs in follow ups |
This is a rewrite of #13774 on top of #13728
Refreshes ConfigurationScriptSource and Playbooks for Ansible Tower
@miq-bot add_label providers/ansible_tower
cc @jameswnl @Ladas @blomquisg