Idea to fix issue #5310 - Using site packages of a different python version for actionrunner #5388
Conversation
idea how to use site-package of custom python version for action runners.
pull-request-size
bot
added
the
size/S
fix lint error
…n_in_action_runner
fix black issue
fix hopefully all black issues now
As python binary for actions defaults to st2 python binary if not explicitly defined in st2 config, the check needs to be adjusted.
…n_in_action_runner
…n_in_action_runner
…thon_version_in_action_runner
There was a problem hiding this comment.
I have a note (for me to avoid future confusion) ... You can ignore that.
And I have a question on how this is supposed to work.
Then, we just need some tests to make sure using a custom python binary for python actions doesn't break with future python updates, and a changelog entry.
| # Add the custom python's site-packages directory in front of the Python | ||
| # system site-packages if python_binary is not default. | ||
| if python_binary is not sys.executable and os.path.isfile(python_binary): | ||
| python_version = python_binary.rsplit("/", 1)[1] |
There was a problem hiding this comment.
Can you give me an example of how this is supposed to work?
eg if someone sets cfg.CONF.actionrunner.python_binary to /usr/local/bin/python for some custom build of python (or whatever), then that path is not going to have a "version" in its string.
There was a problem hiding this comment.
Hi @cognifloyd, as mentioned in the title already this is an idea and I am open for better solutions. I just created it based on what I currently see on my system and found on the internet about python and site packages.
If someone just sets /usr/local/bin/python the variable name python_version might be misleading (I just thought about using different python binaries containing version digits, like python3.7, etc. when I chose the name) but should still to the job, if the site-packages are at the same directories as "usual". Maybe binary_name would be better...
What I have seen so far is that site-packages are usually located at:
/usr/lib/<binary_name>/site-packages
/usr/lib/<binary_name>/dist-packages
/usr/local/lib/<binary_name>/site-packages
/usr/local/lib/<binary_name>/dist-packages
That is the reason why I created system_prefix_dirs and system_dir_names. But it might be that there are linux distributions or custom configs out there using different locations. In this case the solution will not work.
The idea behind these lines was to fix the issue without additional config options as a user might think it should work out of the box when he sets the python_binary config option. This is what I thought as well... ;)
If you want to catch all possibilities, it might be better to implement a new config option like python_binary_site-packages. In this case a user needs to set up two config settings and needs to know where his site packages are located, but he has full control.
Third possibility would be to use both. Look for python_binary_site-packages and if not defined try the lines above.
There was a problem hiding this comment.
Oh yes. binary_name works better.
I like this implementation and how it avoids additional config options.
There was a problem hiding this comment.
Instead of hard-coding the list of site-packages dirs, what if you looked for the dir relative to the python_binary? So Path(python_binary).parent.parent / "lib" / "site-packages" and variants for lib* directories and either site- or dist-
| @@ -137,6 +137,26 @@ def get_sandbox_python_path_for_python_action( | |||
|
|
|||
| pack_base_path = get_pack_base_path(pack_name=pack) | |||
| virtualenv_path = get_sandbox_virtualenv_path(pack=pack) | |||
| python_binary = cfg.CONF.actionrunner.python_binary | |||
There was a problem hiding this comment.
OK. I see that this is already defined. I'll just leave this note here for the next time I review this so I don't have to look it up again.
st2/st2common/st2common/config.py
Lines 454 to 458 in f5c0c0f
…n_in_action_runner
idea how to use site-package of custom python version for action runners.