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

[WIP] Automatically detect python location #42767

Open
wants to merge 1 commit into
base: devel
from

Conversation

Projects
None yet
6 participants
@agaffney
Contributor

agaffney commented Jul 13, 2018

SUMMARY

This PR adds support for automatically detecting the path to python when one is not configured

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

module interpreter

ANSIBLE VERSION
ansible 2.7.0.dev0 (which_python bc6398cbc2) last updated 2018/07/13 15:23:26 (GMT -500)
ADDITIONAL INFORMATION

N/A

lib/ansible/plugins/action/__init__.py Outdated
# necessarily an error.
if result['rc'] <= 1 and len(result['stdout_lines']) > 0:
task_vars['ansible_' + interpreter + '_interpreter'] = result['stdout_lines'][0]
# FUTURE: refactor this along with module build process to better encapsulate "smart wrapper" functionality
(module_style, shebang, module_data, module_path) = self._configure_module(module_name=module_name, module_args=module_args, task_vars=task_vars)

This comment has been minimized.

@bcoca

bcoca Jul 13, 2018

Member

i would execute AFTER getting module's shebang so you can find the 'right interpreter' also check that ansible_x_interpreter does not exist already

This comment has been minimized.

@agaffney

agaffney Jul 13, 2018

Contributor

There's a TODO in there already for finding the correct interpreter for the module, and I've already pushed another commit that adds the check for an existing ansible_x_interpreter var

@s-hertel s-hertel removed the needs_triage label Jul 13, 2018

@nitzmahone

LOTS of stuff to figure out here:

  • non-POSIX
  • performance
  • transparency/troubleshooting
  • interactions with existing ansible_X_interpreter

Those are just off the top of my head...

lib/ansible/plugins/action/__init__.py Outdated
# TODO: parse interpreter from module
interpreter = 'python'
# TODO: make list of binary names to search for configurable on a per-interpreter basis
result = self._low_level_execute_command('which python python3', sudoable=False)

This comment has been minimized.

@nitzmahone

nitzmahone Jul 13, 2018

Member

This will go bang on anything not POSIX (cough Windows cough)... This behavior, or at least the creation of the command to do it, probably needs to be delegated to the shell plugin- no shell-isms or platform-isms in the ActionBase.

This comment has been minimized.

@nitzmahone

nitzmahone Jul 13, 2018

Member

Also, this adds a new round-trip for every module exec (at a time when we're already thrashing around trying to improve performance and exec overhead)... There's gotta be a saner way to do this only when necessary; like an upfront check that gets stored, or kickedoff once in response to a module exec failure, and a way to cache it.

This comment has been minimized.

@agaffney

agaffney Jul 13, 2018

Contributor

The intention is to add a per-host per-interpreter cache for these, so that it's not an ongoing performance hit.

@agaffney agaffney force-pushed the agaffney:which_python branch Jul 13, 2018

@bcoca

This comment has been minimized.

Member

bcoca commented Jul 13, 2018

the big problem is caching, since this is 'post fork' and we really want this to be global

@agaffney agaffney force-pushed the agaffney:which_python branch Jul 13, 2018

@abadger

This comment has been minimized.

Member

abadger commented Jul 13, 2018

Also

  • which is not portable.

IIRC (but you should definitely check this first), I think type is specified by POSIX so it can be used.

@agaffney agaffney force-pushed the agaffney:which_python branch Jul 13, 2018

@bcoca

This comment has been minimized.

Member

bcoca commented Jul 13, 2018

  • skip for binary modules

@agaffney agaffney force-pushed the agaffney:which_python branch 3 times, most recently Jul 13, 2018

@agaffney agaffney force-pushed the agaffney:which_python branch to d5aca75 Jul 14, 2018

@ansible ansible deleted a comment from ansibot Jul 14, 2018

@ansibot ansibot added the test label Jul 14, 2018

@agaffney

This comment has been minimized.

Contributor

agaffney commented Jul 15, 2018

I've put together an alternate approach in #42783

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment