-
Notifications
You must be signed in to change notification settings - Fork 163
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 new python module (explicit) and ansible collections #92
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @kladiv - I don't think we're interested in adding all of these collections to the awx-ee, we've intentionally kept it relatively slim (see #126 (comment)).
That said, I think we'd accept a patch to add ansible.utils
(which will pull in netaddr
as a dep).
I think we'd also add a patch to add in google-cloud-storage
as a pip dep.
Would you be willing to update this PR to just include those two things?
Hi @relrod, I guess most of the included python modules are required by Ansible modules of those collections.
Without some python modules, there should be some Ansible collections modules won't work. Isn't it? To keep AWX EE slim, maybe additional collections could be removed? |
@kladiv necessary deps should automatically get installed with the collections. If there's something you've noticed missing, it's likely something that should be reported to the collections. For example, the azure collection has https://github.com/ansible-collections/azure/blob/dev/meta/execution-environment.yml to point to its list of required python deps and ansible-builder should resolve/install these. |
130830f
to
a23672d
Compare
Hi @relrod, I suggest you check internally if it's strictly required that all those collections should be already included in the base AWX EE or you can keep it slim removing some external collections. To resolve #90 and #91 i changed this PR to add only:
For some reason PR is closed. Are you able to check and reopen it? |
@kladiv First, thanks so much for your work on this! We've not gotten any reports that I'm aware of about these collections being broken due to missing deps. Do you have a specific example or two of something that doesn't work from some collection we include in the current EE? That file I linked is one way that ansible-builder can resolve dependencies. It can also look at a repo-root-level
Researching a bit more -- the google.cloud collection has a
This might have been something the collection fixed since #91 was opened. But I don't think we actually need that change anymore. So that leaves netaddr... I think we could do one of two things here. Just adding So then - we could discuss pulling in the There's also https://github.com/ansible/network-ee which already has So to that end, I think my suggestion would be: Let's add I think I would be inclined to approve that PR, because I think the things in I might bounce it off @shanemcd for a final +1 after that, just to make sure I'm not crazy, though 😉 |
Hi @kladiv Are you able to make the changes above? I'd like to get this PR out of the queue. |
Going to close this for now, feel free to open a new PR with the requested changes above. Thanks! |
Hi @shanemcd.
this PR should resolve #90 and #91.
In addition: