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

Ziploader "recursive imports" and caching #15344

Merged
merged 10 commits into from
Apr 13, 2016
Merged

Conversation

abadger
Copy link
Contributor

@abadger abadger commented Apr 8, 2016

ISSUE TYPE
  • Feature Pull Request
SUMMARY

The first version of ziploader is able to find module_utils files that are listed in the module itself and make sure that those files are included in the zipfile that is sent to the remote machine. This is suboptimal as the equivalent python import will also find the module_utils files that the module_utils files have a need for. This set of commits adds that capability to ziploader and implements a simple controller-side cache to offset the increase in time that searching for the module_utils causes. The cache cleans itself up and auto-invalidates when the main ansible/ansible-playbook process ends.

Caveats to the recusive feature:

  • relative imports (py2.6+) are not handled (example: from . import urls). Imports must be absolute imports coming from ansible.module_utils.
  • python packages (directories with __init__.py in them). module_utils will only be searched for *.py files right now, not directories. (example: from ansible.module_utils.basic import AnsibleModule will look for ansible/module_utils/basic.py. It will not look for ansible/module_utils/basic/__init__.py even though python itself would.)

There's also a couple unrelated bug fixes (Fixing encoding string in the wrapper which was being lost when we implemented comment stripping of the wrapper and a correction to the process.worker.run documentation since worker no longer loops).

@abadger
Copy link
Contributor Author

abadger commented Apr 11, 2016

New commits make these changes, also needed for 2.1:

  • Pass args via stdin (from the wrapper to the module itself) instead of via the environment. This addresss concerns that there's a maximum size to the environment that we may run into.
  • Invoke the module as a script instead of via -m. On python-2.4, -m was implemented very differently and had less features.
    • python-2.4, -m basically means "Look for this python module (not package) in the directories (not zip files or other loaders) of sys.path. If found, run the module as if it were a script. Note that python-2.5 has a -m that allows all of this but is buggy when used against a python package and so python-2.6 removes -m's working with packages: http://stackoverflow.com/questions/4050120/execute-an-installed-python-package-as-a-script which means in python-2.6 and the following directory layout:
|- site-packages
+-|- ansible
  +--- __init__.py
  +--- __main__.py

This does not work: python -m ansible but this does python -m ansible.__main__. (and while the same "works" in pyhton-2.5, it causes some other issues.)

In python-2.7+ they apparently fixed the problems with this so that using -m with a package works.

Anyhow... the fact that python-2.4's -m is just a fancy method of locating a script, since we control where the script gets placed, and since python-2.6 is the first point where we have a -m that we can make do what we want, I decided to just invoke a script for now. When we can say that py2.6 is the lowest version of python we support on the managed node side we can re-add -m support.

This makes our recursive, ast.parse performance measures as fast as
pre-ziploader baseline.

Since this unittest isn't testing that the returned module data is
correct we don't need to worry about os.rename not having any module
data.  Should devise a separate test for the module and caching code
Extract the module and invoke it as a script to work around python-2.4's
lack of features.
@abadger
Copy link
Contributor Author

abadger commented Apr 12, 2016

Latest version has passed travis in my repo: https://travis-ci.org/abadger/ansible/builds/122534682 Ready for review.

@Qalthos Qalthos mentioned this pull request Apr 13, 2016
@abadger abadger merged commit 208ad36 into ansible:devel Apr 13, 2016
@abadger abadger deleted the ziploader branch April 13, 2016 17:27
@abadger
Copy link
Contributor Author

abadger commented Apr 13, 2016

At today's meeting, decided to merge and use devel to test that everything works.

@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant