1480912 - Allow Talos suites to request python3, and propagate that information to the suite's script

Status: RESOLVED WONTFIX

(Testing :: Talos, enhancement)

Description

[(No longer employed by Mozilla) Aaron Klotz]

I've got some new xperf code that is written in Python 3. Rather than backporting it, I'd rather just add facilities to Talos that allow us to optionally run python 3 when needed.

Comment 1

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Add require_py3 option to talos suite configuration and, when set, install python3 and propagate its venv to the test suite

Comment 2

[Robert Wood [:rwood]]

Comment on attachment 8998264 [details] [diff] [review]
Add require_py3 option to talos suite configuration and, when set, install python3 and propagate its venv to the test suite

Review of attachment 8998264 [details] [diff] [review]:
-----------------------------------------------------------------

I don't think this is the way to go here. This patch will force a python 3 venv to be created on every talos windows job. Also, mitmproxy tests (talos tp6) already uses a python 3 venv on Windows. It is invoked/setup via the 'setup_mitmproxy' mozharness step:

https://searchfox.org/mozilla-central/rev/f0c15db995198a1013e1c5f5b5bea54ef83f1049/testing/mozharness/mozharness/mozilla/testing/talos.py#457

We do it that way so that the python 3 venv is only created when mitmproxy is required (tp6). With this patch, and running tp6, two python 3 virtual envs would be created.

From experience it is a pain using a python 3 venv with talos (for tp6) and it has broken in the past, and adds a bunch of setup time to the test jobs. I would really recommend only using a python 3 venv if absolutely necessary - and if so, only call 'setup_py3_virtualenv' if xperf on windows is being run. Thanks!

Also asking for some feedback from :jmaher as he's more familiar with xperf/xtalos than I am.

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8998264 [details] [diff] [review]
Add require_py3 option to talos suite configuration and, when set, install python3 and propagate its venv to the test suite

Review of attachment 8998264 [details] [diff] [review]:
-----------------------------------------------------------------

I don't see a lot of concern with these changes in general.

I would prefer we run all of xperf/talos in python 3, or backport the code to python 2.  as we run talos in python3 for tp6, this might not be too overwhelming to solve.

Comment 4

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to Robert Wood [:rwood] from comment #2)
> Comment on attachment 8998264 [details] [diff] [review]
> Add require_py3 option to talos suite configuration and, when set, install
> python3 and propagate its venv to the test suite
> 
> Review of attachment 8998264 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I don't think this is the way to go here. This patch will force a python 3
> venv to be created on every talos windows job. Also, mitmproxy tests (talos
> tp6) already uses a python 3 venv on Windows. It is invoked/setup via the
> 'setup_mitmproxy' mozharness step:

My patch specifically addresses this: the python3 setup step returns early unless either mitmproxy is needed or 'require_py3' is set in the suite's configuration.

> From experience it is a pain using a python 3 venv with talos (for tp6) and
> it has broken in the past, and adds a bunch of setup time to the test jobs.
> I would really recommend only using a python 3 venv if absolutely necessary
> - and if so, only call 'setup_py3_virtualenv' if xperf on windows is being
> run. Thanks!

Yep, that's what the patch does.

(In reply to Joel Maher ( :jmaher ) (UTC+2) from comment #3)
> I would prefer we run all of xperf/talos in python 3, or backport the code
> to python 2.  as we run talos in python3 for tp6, this might not be too
> overwhelming to solve.

I agree that ideally we wouldn't need to deal with this. I'll see how hard it is to backport my new stuff; I might be able to make it work with some __future__ imports or something.

Comment 5

[Robert Wood [:rwood]]

(In reply to Aaron Klotz [:aklotz] from comment #4)

Ah yes excellent, I missed your 'def query_need_py3(self):', cool. Well looks like it's ok to use py3 if need be, thanks.

Comment 6

[(No longer employed by Mozilla) Aaron Klotz]

Good news! I've backported my new xperf analyzer to python2, so we can drop this bug.