-
Notifications
You must be signed in to change notification settings - Fork 64
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
refactor lookup plugins using top down method #523
refactor lookup plugins using top down method #523
Conversation
Build succeeded. ✔️ ansible-test-cloud-integration-vmware-rest SUCCESS in 13m 56s |
Build failed. ❌ ansible-test-cloud-integration-vmware-rest RETRY_LIMIT in 3m 49s |
Build failed. ❌ ansible-test-cloud-integration-vmware-rest RETRY_LIMIT in 3m 50s |
recheck |
Build succeeded. ✔️ ansible-test-cloud-integration-vmware-rest SUCCESS in 14m 22s |
Heres my before and after playbook. I tried to include it in the PR title comment but that seemed to make the Zuul checks fail. I think its because it tried to parse the ansible vars included in the playbook snippet 🤷 Test Playbook:
Old Output (v4.0.0)
New Output
|
@mariolenz can you review this PR please? |
Build succeeded. ✔️ ansible-test-cloud-integration-vmware-rest SUCCESS in 13m 23s |
Build succeeded. ✔️ ansible-test-cloud-integration-vmware-rest SUCCESS in 15m 57s |
Build succeeded (gate pipeline). ✔️ ansible-test-cloud-integration-vmware-rest SUCCESS in 11m 01s |
7ee65d2
into
ansible-collections:main
SUMMARY
This change rewrites the lookup plugin logic to use a top down approach to finding objects. The plugins will now step through an objects path and try to lookup each intermediate object until it gets to the final object.
The old method has a few issues:
/datacenter/vms/my-folder
and/datacenter/hosts/my-folder
both exist, you can never find either one/datacenter/vms/foo
and/datacenter/hosts/foo
existing at the same time makes it difficult to search for either folderFixes:
#500
#445
#324
ISSUE TYPE
COMPONENT NAME
all lookup plugins
ADDITIONAL INFORMATION
The original top down approach is described in #445. I tried to get that version working but ran into a few issues.