Rescue Python path code #5142

blag · 2021-02-08T10:53:36Z

This PR partially reverts the code removed in #5100 and refactors it a bit since we no longer need to deal with Python 2.7 at all.

In StackStorm <= v3.3, when the sandboxing code was generating a PYTHONPATH for Python actions, it would prepend the operating system's Python directory (eg: /usr/lib/python3.5) in an attempt to provide modules and packages from the Python 3 stdlib before the third party packages (usually backports of the same name) for Python 2.7 (eg: <st2root>/st2/lib/python2.7/site-packages).

All of that was over-enthusiastically ripped out when we removed Python 2.7 support, however some of the PYTHONPATH handling for Python actions is still necessary.

With this PR, the code once again generates an appropriate PYTHONPATH for Python actions:

The pack's virtualenv lib directory (eg: <st2root>/virtualenvs/<pack>/lib/python3.x)
The pack's virtualenv site-packages directory (eg: <st2root>/virtualenvs/<pack>/lib/python3.x/site-packages)
The pack action lib directory (eg: <st2root>/packs/<pack>/actions/lib)
The Python path (optionally) inherited from the parent process' PYTHONPATH, or from the parent virtualenv (the code is only ever called with both set to True), so StackStorm's system site-packages directory will always be appended (eg: <st2root>/st2/lib/python3.x/site-packages)

As we no longer support Python 2.7, we do not need to prepend the PYTHONPATH with the operating system's Python 3 path.

I have also resurrected the tests for this function and refactored them as well.

This might also fix the Windows end-to-end tests that started to fail a few weeks ago.

Related issues and PRs:

Fixes #5126. Special thanks for @amanda11 for catching that before the v3.4 release!

This is currently a WIP. I still need to tweak the tests a bit.

blag · 2021-02-08T10:57:24Z

I created a debug pack with a pythonpath action to see what the differences were between ST2 v3.3 and the unfixed v3.4dev. Here were the results:

StackStorm v3.3 stable on Ubuntu Xenial

.
id: 6020fde0365119859acc1223
action.ref: debug.pythonpath
context.user: st2admin
parameters: None
status: succeeded
start_timestamp: Mon, 08 Feb 2021 09:01:20 UTC
end_timestamp: Mon, 08 Feb 2021 09:01:20 UTC
result:
  exit_code: 0
  result:
    PYTHONPATH:
    - /usr/lib/python3.5
    - /opt/stackstorm/virtualenvs/debug/lib/python3.5
    - /opt/stackstorm/virtualenvs/debug/lib/python3.5/site-packages
    - /opt/stackstorm/packs/debug/actions/lib/
    - ''
    - /opt/stackstorm/st2/lib/python2.7/site-packages
    sys.path:
    - /opt/stackstorm/virtualenvs/debug/lib/python3/dist-packages
    - /opt/stackstorm/st2/lib/python2.7/site-packages/python_runner
    - /usr/lib/python3.5
    - /opt/stackstorm/virtualenvs/debug/lib/python3.5
    - /opt/stackstorm/virtualenvs/debug/lib/python3.5/site-packages
    - /opt/stackstorm/packs/debug/actions/lib
    - /
    - /opt/stackstorm/st2/lib/python2.7/site-packages
    - /opt/stackstorm/virtualenvs/debug/lib/python35.zip
    - /opt/stackstorm/virtualenvs/debug/lib/python3.5/plat-x86_64-linux-gnu
    - /opt/stackstorm/virtualenvs/debug/lib/python3.5/lib-dynload
    - /opt/stackstorm/packs/debug/actions
  stderr: ''
  stdout: ''

StackStorm v3.3 stable on Ubuntu Bionic

.
id: 6020e00ebac336e20eddae5a
action.ref: debug.pythonpath
context.user: st2admin
parameters: None
status: succeeded
start_timestamp: Mon, 08 Feb 2021 06:54:06 UTC
end_timestamp: Mon, 08 Feb 2021 06:54:07 UTC
result:
  exit_code: 0
  result:
    PYTHONPATH:
    - /usr/lib/python3.6
    - /opt/stackstorm/virtualenvs/debug/lib/python3.6
    - /opt/stackstorm/virtualenvs/debug/lib/python3.6/site-packages
    - /opt/stackstorm/packs/debug/actions/lib/
    - ''
    - /opt/stackstorm/st2/lib/python3.6/site-packages
    sys.path:
    - /opt/stackstorm/virtualenvs/debug/lib/python3/dist-packages
    - /opt/stackstorm/st2/lib/python3.6/site-packages/python_runner
    - /usr/lib/python3.6
    - /opt/stackstorm/virtualenvs/debug/lib/python3.6
    - /opt/stackstorm/virtualenvs/debug/lib/python3.6/site-packages
    - /opt/stackstorm/packs/debug/actions/lib
    - /
    - /opt/stackstorm/st2/lib/python3.6/site-packages
    - /opt/stackstorm/virtualenvs/debug/lib/python36.zip
    - /opt/stackstorm/virtualenvs/debug/lib/python3.6/lib-dynload
    - /opt/stackstorm/packs/debug/actions
  stderr: ''
  stdout: ''

StackStorm v3.4dev on Ubuntu Bionic (unfixed)

.
id: 6020f103210e51935211ac3a
action.ref: debug.pythonpath
context.user: st2admin
parameters: None
status: succeeded
start_timestamp: Mon, 08 Feb 2021 08:06:27 UTC
end_timestamp: Mon, 08 Feb 2021 08:06:28 UTC
result:
  exit_code: 0
  result:
    PYTHONPATH:
    - /opt/stackstorm/st2/lib/python3.6/site-packages
    sys.path:
    - /opt/stackstorm/virtualenvs/debug/lib/python3.6/site-packages
    - /opt/stackstorm/st2/lib/python3.6/site-packages/python_runner
    - /opt/stackstorm/st2/lib/python3.6/site-packages
    - /opt/stackstorm/virtualenvs/debug/lib/python36.zip
    - /opt/stackstorm/virtualenvs/debug/lib/python3.6
    - /opt/stackstorm/virtualenvs/debug/lib/python3.6/lib-dynload
    - /usr/lib/python3.6
    - /opt/stackstorm/virtualenvs/debug/lib/python3.6/site-packages
    - /opt/stackstorm/packs/debug/actions
  stderr: ''
  stdout: ''

Kami · 2021-02-12T15:47:17Z

I would also hope that with getting rid of Python 2.7 we will be able to simplify that PYTHONPATH logic, but looks like that is sadly not the case :/

amanda11 · 2021-02-12T15:49:32Z

I believe there is the intention to encourage everyone to use python3 relative imports but we can't do that when we've not given anyone any warning!
So decided we had to keep that logic in for the moment.

Kami

LGTM, although for anything more I would need to dig in (I know that code had various edge cases, for which we did have tests, but yeah removing Python 2.7 support probably exposed some more).

Kami · 2021-02-12T15:51:04Z

@amanda11 Thanks for the additional context.

I do hope that in the future, we will be able to simplify some of that code (although, based on this PR that would mean the change would likely not be fully backward compatible :/)

blag · 2021-02-12T20:37:43Z

Once we force packs to use explicit relative imports instead of implicit relative imports (this is a change made by Python 3 as well), we can simplify some of this a tiny bit.

blag · 2021-02-12T22:02:25Z

Closes #5127 as well.

blag added 4 commits February 8, 2021 02:35

Bring back old sandboxing code

67eec9c

Simplify the sandboxing code a bit

7fe8172

Bring back some of the sandboxing test code

de98293

Refactor some of the test code

c8fd276

blag added bug regression python3 runners labels Feb 8, 2021

blag added this to the 3.4.0 milestone Feb 8, 2021

pull-request-size bot added the size/M PR that changes 30-99 lines. Good size to review. label Feb 8, 2021

Ignore Python 2.7 now

9b2caee

pull-request-size bot added size/L PR that changes 100-499 lines. Requires some effort to review. and removed size/M PR that changes 30-99 lines. Good size to review. labels Feb 9, 2021

blag force-pushed the rescue-pythonpath-code branch from d8a127e to 8188f25 Compare February 9, 2021 11:34

Add an assertEndsWith assertion method and use it

e8327f9

blag force-pushed the rescue-pythonpath-code branch from 8188f25 to e8327f9 Compare February 11, 2021 09:35

blag added 4 commits February 11, 2021 13:08

Silence a completely unrelated file

4087e53

Set maxDiff = None

5f66c49

Convert the PYTHONPATH string to a list before comparing

f9c363e

Fix assertions

74fb938

blag force-pushed the rescue-pythonpath-code branch from fa3ce02 to 74fb938 Compare February 12, 2021 00:16

blag added 5 commits February 11, 2021 18:02

Check that the pack virtualenv exists before trying to list its contents

25bca88

Reformat comment

302d550

Split up test_get_sandbox_python_path_for_python_action function

5c3c624

Use f-strings

c244946

Properly mock os.environ

7bdb137

blag marked this pull request as ready for review February 12, 2021 10:07

Merge branch 'master' into rescue-pythonpath-code

12983fe

Kami approved these changes Feb 12, 2021

View reviewed changes

Merge branch 'master' into rescue-pythonpath-code

2823dc8

blag merged commit 1c2ed13 into master Feb 12, 2021

blag deleted the rescue-pythonpath-code branch February 12, 2021 21:58

blag mentioned this pull request Feb 12, 2021

Fix implicit relative imports in pack actions #5127

Closed

blag changed the title ~~[WIP] Rescue Python path code~~ Rescue Python path code Feb 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Rescue Python path code #5142

Rescue Python path code #5142

Uh oh!

blag commented Feb 8, 2021 •

edited

Loading

Uh oh!

blag commented Feb 8, 2021

Uh oh!

Kami commented Feb 12, 2021

Uh oh!

amanda11 commented Feb 12, 2021

Uh oh!

Kami left a comment

Uh oh!

Kami commented Feb 12, 2021

Uh oh!

blag commented Feb 12, 2021

Uh oh!

blag commented Feb 12, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Rescue Python path code #5142

Rescue Python path code #5142

Uh oh!

Conversation

blag commented Feb 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

blag commented Feb 8, 2021

StackStorm v3.3 stable on Ubuntu Xenial

StackStorm v3.3 stable on Ubuntu Bionic

StackStorm v3.4dev on Ubuntu Bionic (unfixed)

Uh oh!

Kami commented Feb 12, 2021

Uh oh!

amanda11 commented Feb 12, 2021

Uh oh!

Kami left a comment

Choose a reason for hiding this comment

Uh oh!

Kami commented Feb 12, 2021

Uh oh!

blag commented Feb 12, 2021

Uh oh!

blag commented Feb 12, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

blag commented Feb 8, 2021 •

edited

Loading