funsize-balrog-submitter.py not picking up .pth for extra library paths

RESOLVED FIXED

Status

Release Engineering
General Automation
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: sfraser, Assigned: sfraser)

Tracking

unspecified

Firefox Tracking Flags

(firefox52 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Assignee)

Description

a year ago
When making changes for bug 1312433, funsize-balrog-submitter.py could not import the new balrogclient module, despite it being present. 

On investigation, this is because https://dxr.mozilla.org/mozilla-central/source/release/docker/funsize-balrog-submitter/scripts/funsize-balrog-submitter.py#13

  sys.path.insert(0, os.path.join(
      os.path.dirname(__file__), "/home/worker/tools/lib/python"))

is just adding a regular path, not a site directory to sys.path, and .pth files are not processed outside a site directory.

It works later on because of https://dxr.mozilla.org/mozilla-central/source/release/docker/funsize-balrog-submitter/scripts/funsize-balrog-submitter.py#17 and the util module calls 'site.addsitedir(os.path.join(os.path.dirname(__file__), ".."))' which triggers the processing of the .pth file.

The shortest code change to make it work would be putting the `util` import before the others, but that leaves the mechanism as feeling slightly magic when reading the script. 

Code review coming in shortly for a change from sys.path.insert to a site.addsitedir() call, to make it explicitly clear what's happening, and so that the .pth file processing is not magic when reading the script.

The `os.path.dirname(__file__)` component doesn't appear to do anything useful, so will be removed. The absolute path as the second component means the first argument to os.path.join is ignored.
Comment hidden (mozreview-request)

Comment 2

a year ago
mozreview-review
Comment on attachment 8806670 [details]
bug 1314574 replace sys.path with site.addsitedir so that .pth files are processed.

https://reviewboard.mozilla.org/r/90046/#review89606

I tested the patch using the following scenario:

* apply the current patch to mozilla-inbound
* modified Dockerfile to clone the tools repo by revision: `RUN hg clone -r c41157cfe717 https://hg.mozilla.org/build/tools /home/worker/tools`
* built the image
* entered the image and tried to run the script. It didn't complain about `ImportError` \o/

We need to land this patch on all branches to make sure our release automation is ready for the main patch, then regenerate the nightly funsize docker image, and only after these steps land the tools patch.

I can land this patch with a=release.
Attachment #8806670 - Flags: review?(rail) → review+

Comment 3

a year ago
Pushed by raliiev@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1caaca1ab3f
replace sys.path with site.addsitedir so that .pth files are processed.  r=rail a=release DONTBUILD
(In reply to Rail Aliiev [:rail] ⌚️ET from comment #2)

> regenerate the nightly funsize docker image, and only after these steps land the tools patch.

Err, the other way around. Land the tools patch, then regenerate.
Assignee: nobody → sfraser
Comment hidden (mozreview-request)
(Assignee)

Comment 7

a year ago
mozreview-review
Comment on attachment 8806691 [details]
Bug 1314574 - Bump funsize version

https://reviewboard.mozilla.org/r/90058/#review89616
Attachment #8806691 - Flags: review?(sfraser) → review+

Comment 9

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e1caaca1ab3f
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.