Skip to content
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

Add lookup plugin to use native api #136

Merged
merged 8 commits into from
Dec 2, 2024

Conversation

tanganellilore
Copy link
Contributor

@tanganellilore tanganellilore commented Sep 19, 2024

Hi team,

In this PR I added the functionality to use native API via lookup plugin.
I'm simply inspired from AWX controller_api so the implementation it's very simple and without special checks.

Lookup working like controller_apu lookup, ex:

- name: List all Virtual Machines from the API
  set_fact:
    virtual_machines: "{{ lookup('ngine_io.cloudstack.api', 'listVirtualMachines') }}"

- name: List specific Virtual Machines from the API
  set_fact:
    virtual_machines: "{{ lookup('ngine_io.cloudstack.api', 'listVirtualMachines', query_params={ 'name': 'myvmname' }) }}"

This is very usefull for usecase like listing elements or simply query the api, without any logic that generally is present on a module (like automatic retrival of id from a name).

Let me know it this can be a good starting point.

Thanks

@tanganellilore
Copy link
Contributor Author

@resmo have you see that?

@resmo resmo self-requested a review October 28, 2024 12:09
@resmo
Copy link
Member

resmo commented Oct 28, 2024

Hi @tanganellilore

Thanks for pinging me. I added myself as reviewer.

In general, I agree with this functionality. However, I'm in the middle of a major refactoring (see #141) that will be done by about the end of the year. Would it be ok to take a look at this PR when the dust has settled a bit?

@tanganellilore
Copy link
Contributor Author

Hi @resmo ,

no problem, in the meantime i will use my fork with this enhancement.
let me say, is very, very simple, I create a litte wrapping to cs library, without adding logic or something similar

Copy link
Member

@resmo resmo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your suggestion. I'd like to have this lookup plugin merged for 3.0.0. However, I wonder why you created another module_utils for it. Can you explain?

plugins/lookup/api.py Outdated Show resolved Hide resolved
@resmo
Copy link
Member

resmo commented Dec 2, 2024

@tanganellilore please rebase

@tanganellilore
Copy link
Contributor Author

Hi,

to due different init and api function, moreover, to separate lookup/api module to other module.

Api lookup is very flexible without any specific check.

@resmo
Copy link
Member

resmo commented Dec 2, 2024

sorry again, could you please rebase again git rebase upstream/master then force push git push -f origin add_api_lookup again. Thanks.

plugins/lookup/api.py Outdated Show resolved Hide resolved
@resmo
Copy link
Member

resmo commented Dec 2, 2024

LGTM, I may re-arrange code a bit but it's good for now.

@resmo resmo merged commit ca0a634 into ngine-io:master Dec 2, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants