1189021 - talos is not seen as a package when used by mozharness

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

[Julien Pagès (:parkouss)]

talos is not seen as a package when used by mozharness. Because of that, we can't use imports statements like:

from talos import foo
import talos.bar

this will only works locally - and break on try with ImportError.

We should fix that, and then use relative import inside talos to avoid name conflicts with python files outside talos.

Comment 1

[Julien Pagès (:parkouss)]

Attachment: 1189021.patch

This should fix the issue.

So what it does:

- install talos in the virtualenv (instead of just the dependencies). as pip will install talos dependencies automatically, this makes the code a lot easier.

- run the talos executable that has been created by pip in the virtualenv bin directory. (this is the entrypoint "talos" in setup.py). We ensure this way that the good python will be used, and that the right talos will be used.


So this implies (if that works, but I know it will!) that from now on we should always use the binary "talos" installed with pip instead of manually running the run_tests.py file. Because this will be how mozharness does that anyway, and this is safer and easier (and trust me, this is a good thing. :)).


This works fine locally.

Note that for simplicity I removed the hack for yaml, so we should wait for bug 1188756 before landing that.

Comment 2

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

Comment on attachment 8640970 [details] [diff] [review]
1189021.patch

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

we had issues with installing talos as a package previously on some newly imaged linux64 machines.  It would show up about once a week and we effectively had to pull the machines out of production.

Comment 3

[Julien Pagès (:parkouss)]

Well, maybe editable mode can help us here. I asked :jlund to have his opinion.

Comment 4

[Julien Pagès (:parkouss)]

Attachment: 1189021.patch

Ok, patch updated with what I said in bug 1112773 comment 55.

Comment 5

[Jordan Lund (:jlund)]

Comment on attachment 8641055 [details] [diff] [review]
1189021.patch

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

interesting, I didn't know about editable. My only issue here is that we still haven't solved bug 1141416 which we thought was related to bug 1112773.

I think before we start changing the process for installing talos again, we should determine what is happening in 1141416.

jmaher, do you agree?

Comment 6

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

yeah, I agree we should figure out the root cause, although I didn't really understand what is still broken after looking at the loaner slave.  A lot is changing and we are moving talos in tree.  

Ideally once we are in-tree and we all feel comfortable that talos can be updated appropriately, then I would be up for experimenting with talos as a package.  Honestly I think we should focus on making talos run more like mochitest, reftest though.

Comment 7

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

Comment on attachment 8641055 [details] [diff] [review]
1189021.patch

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

while this looks great code wise, I am not convinced this is the route we really want to go.

Comment 8

[Julien Pagès (:parkouss)]

Right. Then let's do that using PYTHONPATH then. :)

Comment 9

[Julien Pagès (:parkouss)]

Attachment: 1189021_2.patch

Comment 10

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

Comment on attachment 8642267 [details] [diff] [review]
1189021_2.patch

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

this looks good!

Comment 11

[Julien Pagès (:parkouss)]

Pushed to try:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9b1b7b182f51

Comment 12

[Julien Pagès (:parkouss)]

leave-open until we fixes internal talos imports.

Comment 13

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/887432a3049f

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/887432a3049f

Comment 15

[Julien Pagès (:parkouss)]

So I changed my mind - closing this now, we will fix imports later if needed.

Comment 16

[Firefox Bug Husbandry Bot]

Removing leave-open keyword from resolved bugs, per :sylvestre.