patch st2-run-pack-tests to work with python3 #4956

guzzijones · 2020-05-20T21:11:12Z

allows st2-run-pack-tests to work with python3 on systems that have python3 installed as a separate binary.

I probably need to find and or create an integration test for this.

guzzijones · 2020-05-20T21:34:57Z

I see 2 places where this script is called:
https://github.com/StackStorm/st2/blob/b54262709f02b17acb716ed12efc651c71e1405d/Makefile
https://github.com/StackStorm/st2/blob/2b7c4005243fc268497b6780333241e9fc712f82/tox.ini

Adding a test of the -3 will require a git clone of the repo and setting the ST2_REPO_PATH and then running .

arm4b · 2020-05-23T18:40:45Z

@nzlosh Can you please help evaluating UX functionality/changes in this PR and review it?

nzlosh · 2020-05-24T09:14:28Z

Can I have some more context about the problem this PR is addressing? On Ubuntu 18.04 the st2-run-pack-tests detects and uses python3.6 to test the pack without any problem. Running the script with -3 returns must set ST2_REPO_PATH variable

guzzijones · 2020-05-25T15:32:29Z

The centos7 install uses python2.7. Adding this switch will tell the pack test script to NOT look for the st2 packages in the installed dir at /opt/stackstorm. Yes on ubuntu you do not need this. The ubunu install uses python3. Furthermore, to test this correctly we need to run this script with the new -3 flag on a centos7 box. You cant simply run tests on en empty box. And we cant * that i know of * use the tox.ini because that wont have the versions conflict with python. To reproduce the bug this fixes try to run a python3 action script though the original on Stackstorm centos7 installed box. You will see it try to load up python2.7 libs into its pythonpath. And the test will fail. In the end this is dead end code. But i have to use this version on our st2 installed testing box. I submitted it to help others who run onto the same issue with testing on a stackstorm centos7 box.

…

On Sun, May 24, 2020, 05:14 Carlos ***@***.***> wrote: Can I have some more context about the problem this PR is addressing? On Ubuntu 18.04 the st2-run-pack-tests detects and uses python3.6 to test the pack without any problem. Running the script with -3 returns must set ST2_REPO_PATH variable — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#4956 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACZ5TILXE57SDDDRAMQY25TRTDQQBANCNFSM4NGKB4LQ> .

guzzijones · 2020-05-25T15:34:35Z

Essentially this flag says, "python is 2.7 and python3 is 3 on the system.

…

On Mon, May 25, 2020, 11:32 aj jones ***@***.***> wrote: The centos7 install uses python2.7. Adding this switch will tell the pack test script to NOT look for the st2 packages in the installed dir at /opt/stackstorm. Yes on ubuntu you do not need this. The ubunu install uses python3. Furthermore, to test this correctly we need to run this script with the new -3 flag on a centos7 box. You cant simply run tests on en empty box. And we cant * that i know of * use the tox.ini because that wont have the versions conflict with python. To reproduce the bug this fixes try to run a python3 action script though the original on Stackstorm centos7 installed box. You will see it try to load up python2.7 libs into its pythonpath. And the test will fail. In the end this is dead end code. But i have to use this version on our st2 installed testing box. I submitted it to help others who run onto the same issue with testing on a stackstorm centos7 box. On Sun, May 24, 2020, 05:14 Carlos ***@***.***> wrote: > Can I have some more context about the problem this PR is addressing? On > Ubuntu 18.04 the st2-run-pack-tests detects and uses python3.6 to test > the pack without any problem. Running the script with -3 returns must > set ST2_REPO_PATH variable > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#4956 (comment)>, or > unsubscribe > <https://github.com/notifications/unsubscribe-auth/ACZ5TILXE57SDDDRAMQY25TRTDQQBANCNFSM4NGKB4LQ> > . >

arm4b · 2020-05-25T17:33:16Z

Here is the complete py2 vs py3 native OS support matrix:

Python 3:

EL8
Ubuntu Bionic

Python 2:

EL6 (will be deprecated in st2 v3.3)
EL7
Ubuntu Xenial

arm4b · 2020-05-30T08:38:18Z

@nzlosh did @guzzijones answer your question?
Any further PR review/feedback based on that context?

nzlosh

Overall, adding support for Python3 looks good to me. There are a few points I've raised, such as allowing the users to choose the version of python used for the pack tests and some more explicit error messages.

nzlosh · 2020-05-24T09:33:17Z

st2common/bin/st2-run-pack-tests

@@ -78,10 +80,11 @@ function usage() {
    echo "       and virtualenv, if any, for subsequent test runs." >&2
    echo "  -t : Run tests with timing enabled" >&2
    echo "  -v : Verbose mode (log debug messages to stdout)." >&2
+    echo "  -3 : use python3" >&2


Use a flag that takes an argument rather than hard coding 3 (hint: use $OPTARG to get the version).

3 doesn't give any meaning to the purpose of the flag, using P would be better so there is an association with P for Python. It would be useful to be able to specify a version rather than just the major version. E.g. accept 3.6, 3.7, 3.8 to give pack developers the flexibility to test against a specific minor version.

Remember to document the flag in the Usage: $0 section of the output too.

nzlosh · 2020-05-24T09:36:18Z

st2common/bin/st2-run-pack-tests

@@ -104,6 +107,14 @@ while getopts ":p:f:xjvct" o; do
        t)
            ENABLE_TIMING=true
            ;;
+        3)
+            PYTHON_3=true
+            if [ -z "$ST2_REPO_PATH" ]; then


Why does the python 3 flag short circuit the ST2_REPO_PATH logic which handles the case of its absence later in the script?

This is the most important part. This script will look for st2 libs in \opt\stsckstorm if i dont set this. On centos7 those are python 2.7. Maybe what we do here is check the system python version vs. The version passed in this flag. If they doffer then force this env var to be set.

nzlosh · 2020-05-30T09:51:16Z

st2common/bin/st2-run-pack-tests

+        3)
+            PYTHON_3=true
+            if [ -z "$ST2_REPO_PATH" ]; then
+                echo "must set ST2_REPO_PATH variable"


A clearer error message explaining what the ST2_REPO_PATH variable is used for will help people set the correct value. e.g. Environment variable ST2_REPO_PATH must be set to your st2 installation or st2 git repository for pack tests to function correctly.

guzzijones · 2020-06-01T17:38:19Z

Just a heads up:
If this gets milestone I will spend some time on it. Currently I am working on other PRs.

arm4b · 2020-06-01T19:06:31Z

@guzzijones Yes, we'll be good to accept this enhancement into st2 core, once it's polished.

Thanks @nzlosh for helping with the review to get it closer for the broader use.

arm4b · 2020-08-13T13:29:10Z

@guzzijones any update on driving this PR to completion?

cognifloyd · 2021-07-23T17:08:58Z

We do not support python2 now, so is this needed anymore?

amanda11 · 2021-07-23T17:16:52Z

We do not support python2 now, so is this needed anymore?

I don't think so, the last two ST2 releases (3.4 and 3.5) do not support python2.

cognifloyd · 2021-07-23T19:34:28Z

I'm going to close this then. If we still need it for some reason, feel free to reopen.

arm4b · 2021-07-25T13:41:31Z

Closing due to py2 is not relevant anymore and we're on py3 sounds 👍
Thanks @cognifloyd for spotting this!

patch st2-run-pack-tests to work with python3

3f2a5e5

pull-request-size bot added size/S PR that changes 10-29 lines. Very easy to review. size/M PR that changes 30-99 lines. Good size to review. and removed size/S PR that changes 10-29 lines. Very easy to review. labels May 20, 2020

guzzijones added 3 commits May 20, 2020 21:23

branch around nose options for python3

76e57a3

fixup branch for python 3 for nose opts

244d78e

add initialization of PYTHON_3

7a00220

arm4b added the enhancement label May 23, 2020

nzlosh reviewed May 30, 2020

View reviewed changes

arm4b added WIP status:needs changes and removed WIP labels Aug 13, 2020

arm4b mentioned this pull request Aug 13, 2020

Add AJ Jonen (@guzzijones) to the list of StackStorm Contributors #5025

Merged

cognifloyd closed this Jul 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

patch st2-run-pack-tests to work with python3 #4956

patch st2-run-pack-tests to work with python3 #4956

guzzijones commented May 20, 2020

guzzijones commented May 20, 2020

arm4b commented May 23, 2020

nzlosh commented May 24, 2020

guzzijones commented May 25, 2020 via email

guzzijones commented May 25, 2020 via email

arm4b commented May 25, 2020 •

edited

Loading

arm4b commented May 30, 2020

nzlosh left a comment

nzlosh May 24, 2020

nzlosh May 24, 2020

guzzijones May 30, 2020

nzlosh May 30, 2020

guzzijones commented Jun 1, 2020

arm4b commented Jun 1, 2020

arm4b commented Aug 13, 2020

cognifloyd commented Jul 23, 2021

amanda11 commented Jul 23, 2021

cognifloyd commented Jul 23, 2021

arm4b commented Jul 25, 2021 •

edited

Loading

patch st2-run-pack-tests to work with python3 #4956

patch st2-run-pack-tests to work with python3 #4956

Conversation

guzzijones commented May 20, 2020

guzzijones commented May 20, 2020

arm4b commented May 23, 2020

nzlosh commented May 24, 2020

guzzijones commented May 25, 2020 via email

guzzijones commented May 25, 2020 via email

arm4b commented May 25, 2020 • edited Loading

arm4b commented May 30, 2020

nzlosh left a comment

Choose a reason for hiding this comment

nzlosh May 24, 2020

Choose a reason for hiding this comment

nzlosh May 24, 2020

Choose a reason for hiding this comment

guzzijones May 30, 2020

Choose a reason for hiding this comment

nzlosh May 30, 2020

Choose a reason for hiding this comment

guzzijones commented Jun 1, 2020

arm4b commented Jun 1, 2020

arm4b commented Aug 13, 2020

cognifloyd commented Jul 23, 2021

amanda11 commented Jul 23, 2021

cognifloyd commented Jul 23, 2021

arm4b commented Jul 25, 2021 • edited Loading

arm4b commented May 25, 2020 •

edited

Loading

arm4b commented Jul 25, 2021 •

edited

Loading