Closed Bug 719567 Opened 10 years ago Closed 10 years ago

expand talos.json to support pageloader.xpi

Categories

(Release Engineering :: General, defect, P2)

x86
Linux
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jmaher, Assigned: armenzg)

References

Details

(Whiteboard: [SfN])

Attachments

(3 files, 5 obsolete files)

we have a great method for reducing our needs of releng to deploy minor talos changes via talos.json.  As we forsee a good deal of pageloader changes in the near future, it would be great to have an addition to talos.json for pageloader.xpi.
Whiteboard: [SfN]
Duplicate of this bug: 719574
Assignee: nobody → armenzg
Status: NEW → ASSIGNED
Priority: -- → P2
From the output we can see that the talos.json in my people account works (the talos2.json is to show failing purposely) and it has "downloadables", "talos.zip" and "pageloader.xpi" keys.

This patch will follow a buildbotcustom patch and a talos.json change.

Armens-MacBook-Air:tools armenzg$ python scripts/talos/talos_from_code.py --talos-json-url http://hg.mozilla.org/mozilla-central/raw-file/default/testing/talos/talos.json
WARNING: This talos.json does not have a valid 'downloables' list. Falling back to just download talos.zip
INFO: talos.json URL: http://hg.mozilla.org/mozilla-central/raw-file/default/testing/talos/talos.json
INFO: talos.zip URL: http://build.mozilla.org/talos/zips/talos.bug719946.49138575dae9.zip
Armens-MacBook-Air:tools armenzg$ ls -l talos.zip &&  md5 talos.zip 
-rw-r--r--  1 armenzg  staff  6004166 27 Jan 11:18 talos.zip
MD5 (talos.zip) = 5deee9372109ef6951b07843f18afe67
Armens-MacBook-Air:tools armenzg$ python scripts/talos/talos_from_code.py --talos-json-url http://hg.mozilla.org/projects/pine/raw-file/67962f8c1fdf/testing/talos/talos.json
WARNING: This talos.json does not have a valid 'downloables' list. Falling back to just download talos.zip
INFO: talos.json URL: http://hg.mozilla.org/projects/pine/raw-file/67962f8c1fdf/testing/talos/talos.json
INFO: talos.zip URL: http://people.mozilla.org/~jmaher/taloszips/tip/zips/talos.pine.zip
Armens-MacBook-Air:tools armenzg$ ls -l talos.zip && md5 talos.zip 
-rw-r--r--  1 armenzg  staff  6011515 27 Jan 11:19 talos.zip
MD5 (talos.zip) = df837e1a623ef60815ede2d7245f2d1a
Armens-MacBook-Air:tools armenzg$ python scripts/talos/talos_from_code.py --talos-json-url http://people.mozilla.com/~armenzg/talos/talos2.json
WARNING: This talos.json does not have a valid 'downloables' list. Falling back to just download talos.zip
ERROR: You have tried to download a file from: http://people.mozilla.org/~jmaher/taloszips/tip/zips/talos.pine.zip which is a location different than http://build.mozilla.org/talos/
ERROR: This is only allowed for the certain branches.
Armens-MacBook-Air:tools armenzg$ python scripts/talos/talos_from_code.py --talos-json-url http://people.mozilla.com/~armenzg/talos/talos.json
INFO: talos.json URL: http://people.mozilla.com/~armenzg/talos/talos.json
INFO: talos.zip URL: http://build.mozilla.org/talos/zips/talos.zip
INFO: pageloader.xpi URL: http://build.mozilla.org/talos/xpis/pageloader.xpi
Armens-MacBook-Air:tools armenzg$ ls -l talos.zip && md5 talos.zip 
-rw-r--r--  1 armenzg  staff  6010342 27 Jan 11:19 talos.zip
MD5 (talos.zip) = 35c53949de1f3f58021f852c23f4fc8e
Attachment #592145 - Flags: review?(jmaher)
Comment on attachment 592145 [details] [diff] [review]
enable talos_from_code to download as many files as specified in "downloadables" list

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

so this supports:
1) original talos.json format
2) new talos.json format with downloadables section
3) old way

I see with option #2 we can specify just about anything (talos.zip, pageloader.xpi, others).  This is great, but how do we ensure that the pageloader step doesn't overwrite this?  Will we pass in the --talos-json-url to the pageloader download step?

Otherwise this is great.

::: scripts/talos/talos_from_code.py
@@ +44,5 @@
> +            downloadUrl = get_value(jsonFilename, key)
> +            if passesRestrictions(options.talos_json_url, downloadUrl):
> +                # the key is at the same time the filename e.g. talos.zip
> +                download_file(downloadUrl, key)
> +                print "INFO: %s URL: %s" % (key, downloadUrl)

what about the case where it doesn't pass, or we can't find the key?  Not sure if there should be an else clause or a downloadDefault() call.

@@ +46,5 @@
> +                # the key is at the same time the filename e.g. talos.zip
> +                download_file(downloadUrl, key)
> +                print "INFO: %s URL: %s" % (key, downloadUrl)
> +    else:
> +        print "WARNING: This talos.json does not have a valid 'downloables' list. " + \

typo in downloadables
Attachment #592145 - Flags: review?(jmaher) → review+
(In reply to Joel Maher (:jmaher) from comment #3)
> 
> so this supports:
> 1) original talos.json format
> 2) new talos.json format with downloadables section
> 3) old way
> 
> I see with option #2 we can specify just about anything (talos.zip,
> pageloader.xpi, others).  This is great, but how do we ensure that the
> pageloader step doesn't overwrite this?  Will we pass in the
> --talos-json-url to the pageloader download step?
> 
> Otherwise this is great.
>
I am going to move the current download of the pageloader.xpi file before we call the script. That way we can remove the original one and download the new one.
Either that or we have to backport the new format everywhere.

talos_from_code.py should live in repo as well. That way we don't have to support multiple code paths.

> ::: scripts/talos/talos_from_code.py
> @@ +44,5 @@
> > +            downloadUrl = get_value(jsonFilename, key)
> > +            if passesRestrictions(options.talos_json_url, downloadUrl):
> > +                # the key is at the same time the filename e.g. talos.zip
> > +                download_file(downloadUrl, key)
> > +                print "INFO: %s URL: %s" % (key, downloadUrl)
> 
> what about the case where it doesn't pass, or we can't find the key?  Not
> sure if there should be an else clause or a downloadDefault() call.
Attachment #592145 - Attachment is obsolete: true
Attachment #592194 - Flags: review?(jmaher)
Comment on attachment 592194 [details] [diff] [review]
enable talos_from_code to download as many files as specified in "downloadables" list

I'm rethinking this.
Attachment #592194 - Flags: review?(jmaher)
Depends on: 721822
I can go back to the "downloadables" model if you prefer to.
Attachment #592194 - Attachment is obsolete: true
Attachment #593495 - Flags: review?(jmaher)
The remove step is to clobber the cached version we run since December.
This probably would fix pine not being able to pull from people (not sure if you are still hitting it).

This has to land together with attachment 593495 [details] [diff] [review].
Attachment #593498 - Flags: review?(jmaher)
Comment on attachment 593495 [details] [diff] [review]
[m-c/talos_from_code.py] pageloader.xpi support

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

I like this a lot better than the downloadables.  The use of path really makes me happy!
Attachment #593495 - Flags: review?(jmaher) → review+
Comment on attachment 593498 [details] [diff] [review]
[process/factory.py] rm talos_from_code.py and add pageloader.xpi from talos.json support

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

::: process/factory.py
@@ +7558,5 @@
> +                    workdir=self.workdirBase,
> +                    flunkOnFailure=False,
> +                    warnOnFailure=True,
> +                    haltOnFailure=False,
> +                ))

is this a temporary step?
Attachment #593498 - Flags: review?(jmaher) → review+
Small changes to download_file with "saveAs". Looking at inter-diff makes it easier.

My testing on staging was not correct and this is what actually was being tested and it has been passing.
Attachment #593571 - Flags: review?(jmaher)
I was assuming that we were using a cached version of talos_from_source.py but it actually gets clobbered.
Attachment #593498 - Attachment is obsolete: true
Attachment #593572 - Flags: review?(jmaher)
Attachment #593572 - Flags: review?(jmaher) → review+
Comment on attachment 593571 [details] [diff] [review]
[m-c/talos_from_code.py] pageloader.xpi support

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

::: testing/talos/talos_from_code.py
@@ +100,5 @@
> +        except Exception, e:
> +            print "ERROR: %s" % str(e)
> +            sys.exit(1)
> +    filename = saveAs if saveAs else get_filename_from_url(url)
> +    local_file = open(os.path.join(path, filename), 'wb')

good catch here.
Attachment #593571 - Flags: review?(jmaher) → review+
Attachment #593495 - Attachment is obsolete: true
Comment on attachment 593571 [details] [diff] [review]
[m-c/talos_from_code.py] pageloader.xpi support

[Approval Request Comment]
This is a patch needed to land on m-a at the same as on m-c.
This can only affect talos jobs and would be noticeable immediately in both m-c and m-a.
This has been tested thoroughly.
Attachment #593571 - Flags: approval-mozilla-aurora?
Comment on attachment 593571 [details] [diff] [review]
[m-c/talos_from_code.py] pageloader.xpi support

[Triage Comment]
Approved for Aurora 11.
Attachment #593571 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
*Aurora 12
Comment on attachment 593572 [details] [diff] [review]
593498: [process/factory.py] rm talos_from_code.py and add pageloader.xpi from talos.json support

http://hg.mozilla.org/build/buildbotcustom/rev/23521f390131
Attachment #593572 - Flags: checked-in+
This is live in production since 8:20 AM PDT.
Attached patch support both talos.json formats (obsolete) — Splinter Review
This is interim until all branches support the new format.

The only difference to what we landed on mozilla-central is the new if clause with the first part being:
 47         # XXX: This if clause is interim until all branches support the new talos.json format
 48         url = get_value(jsonFilename, 'talos_zip')
 49         download_file(url, "", 'talos_zip')
 50         print "INFO: %s -> %s" % (url, 'talos_zip')
 51         # NOTE: We're obviously hard-coding
 52         url = 'http://build.mozilla.org/talos/xpis/pageloader.xpi'
 53         download_file(url, "talos/page_load_test", 'pageloader.xpi')
 54         print "INFO: %s -> %s" % (url, 'pageloader.xpi')

If a key is missing we return None.
124     try:
125         return json.load(f)[key]
126     except:
127         return None

From the output below you can see that it works with the new format (m-c, m-a) and the old one (m-i):
Armens-MacBook-Air:tools armenzg$ python scripts/talos/talos_from_code.py --talos-json-url http://hg.mozilla.org/mozilla-central/raw-file/default/testing/talos/talos.json
INFO: talos.json URL: http://hg.mozilla.org/mozilla-central/raw-file/default/testing/talos/talos.json
INFO: http://build.mozilla.org/talos/zips/talos.bug721857.05f01e049452.zip -> talos.zip
INFO: http://build.mozilla.org/talos/xpis/pageloader.xpi -> talos/page_load_test/pageloader.xpi

Armens-MacBook-Air:tools armenzg$ python scripts/talos/talos_from_code.py --talos-json-url http://hg.mozilla.org/releases/mozilla-aurora/raw-file/default/testing/talos/talos.json
INFO: talos.json URL: http://hg.mozilla.org/releases/mozilla-aurora/raw-file/default/testing/talos/talos.json
INFO: http://build.mozilla.org/talos/zips/talos.bug721857.05f01e049452.zip -> talos.zip
INFO: http://build.mozilla.org/talos/xpis/pageloader.xpi -> talos/page_load_test/pageloader.xpi

Armens-MacBook-Air:tools armenzg$ python scripts/talos/talos_from_code.py --talos-json-url http://hg.mozilla.org/integration/mozilla-inbound/raw-file/default/testing/talos/talos.json
INFO: talos.json URL: http://hg.mozilla.org/integration/mozilla-inbound/raw-file/default/testing/talos/talos.json
INFO: http://build.mozilla.org/talos/zips/talos.bug721857.05f01e049452.zip -> talos_zip
INFO: http://build.mozilla.org/talos/xpis/pageloader.xpi -> pageloader.xpi
Attachment #593877 - Flags: review?(jmaher)
I attached the wrong one.

This patch makes it easier to see the delta from what we landed on m-c.
Attachment #593877 - Attachment is obsolete: true
Attachment #593877 - Flags: review?(jmaher)
Attachment #593885 - Flags: review?(jmaher)
Comment on attachment 593885 [details] [diff] [review]
support both talos.json formats

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

ahh, good idea
Attachment #593885 - Flags: review?(jmaher) → review+
Comment on attachment 593885 [details] [diff] [review]
support both talos.json formats

This is live in production:
http://hg.mozilla.org/build/tools/rev/d4eb54621cce
Attachment #593885 - Flags: checked-in+
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Fix to support both talos.json formats 9:46AM PDT:
http://hg.mozilla.org/build/tools/rev/d4eb54621cce

Bustage fix at 13:05PM PDT:
http://hg.mozilla.org/build/tools/rev/ab74fec66359
* because my previous bustage fix was downloading the files as "talos_zip" rather than "talos.zip"

Bustage fix at 14:15PM PDT :
http://hg.mozilla.org/build/buildbotcustom/rev/c468f12b4082
* because wget in Leopard and Windows needs --no-check-certificate

There was a big gap at the beginning because of the lack of logs and three meetings I had in the afternoon.
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.