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

Collection loader: importing from module_utils/foo/__init__.py does not work #68872

Closed
felixfontein opened this issue Apr 11, 2020 · 6 comments · Fixed by #67684
Closed

Collection loader: importing from module_utils/foo/__init__.py does not work #68872

felixfontein opened this issue Apr 11, 2020 · 6 comments · Fixed by #67684
Labels
affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. support:core This issue/PR relates to code supported by the Ansible Engineering Team.

Comments

@felixfontein
Copy link
Contributor

SUMMARY

During some refactoring I tried to rename plugins/module_utils/crypto.py to plugins/module_utils/crypto/__init__.py in community.crypto, assuming that this does not change behavior for any module using this module_utils. Unfortunately, that does not work: while from ansible_collections.community.crypto.plugins.module_utils import crypto as crypto_utils works fine, from ansible_collections.community.crypto.plugins.module_utils.crypto import CRYPTOGRAPHY_HAS_X25519 makes the task fail with:

fatal: [localhost]: FAILED! => {"msg": "Could not find imported module support code for openssl_privatekey.  Looked for either CRYPTOGRAPHY_HAS_ED448.py or crypto.py"}

As @abadger pointed out, there are explicit tests in ansible/ansible which test this for module_utils inside ansible/ansible itself:

There seem to be no similar tests for collection module_utils, as these will apparently fail.

CC @mattclay @nitzmahone

ISSUE TYPE
  • Bug Report
COMPONENT NAME

collection loader

ANSIBLE VERSION
devel
probably also 2.9
@ansibot
Copy link
Contributor

ansibot commented Apr 11, 2020

Files identified in the description:
None

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibot ansibot added affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Apr 11, 2020
@mattclay
Copy link
Member

@felixfontein Non-empty __init__.py files are something we actually test for in the sanity tests because they're not supported. Including one in a collection results in an error:

ERROR: Found 1 empty-init issue(s) which need to be resolved:
ERROR: plugins/module_utils/crypto/__init__.py:0:0: empty __init__.py required

@mattclay mattclay removed the needs_triage Needs a first human triage before being processed. label Apr 13, 2020
@felixfontein
Copy link
Contributor Author

Ah, that's good to know! I never came that far since I didn't try running any tests except integration tests locally, and these already failed because of this.

I've now also found it in the docs: Note that importing something from an ``__init__.py`` file requires using the file name: (https://github.com/ansible/ansible/blob/devel/docs/docsite/rst/dev_guide/developing_collections.rst). Sorry for missing that...

@abadger
Copy link
Contributor

abadger commented Apr 15, 2020

@mattclay i don't think we can do that. Putting things in __init__.py files is a very central feature of python. I've tried to keep things out of __init__.py in core because it prevents using namespace packages down the road, but it serves some very useful purposes, such as the backwards compatibility that felix was working on when he ran across this in the first place.

Since collections are even more outside of the ansible scope than what was in core, it policies won't apply there and people will want to do this even more. So i think this needs to work for collections just as i supported it working in ansible/ansible when i wrote ansiballz

@abadger abadger reopened this Apr 15, 2020
@abadger
Copy link
Contributor

abadger commented Apr 15, 2020

Nitzmahone also acknowledged this as a bug when i talked to him on Monday. He had written an integration test for it but commented it out with a note to fix it later but forgot about it.

@mattclay
Copy link
Member

@abadger I didn't mean to suggest we shouldn't allow it. I only meant to point out that it's currently something that is reported as an error in the empty-init sanity test. Aside from that and the collection loader issue, there may be other areas that don't support it well, at least in tests.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants