1249768 - mozilla-taskcluster is submitting resultsets to Treeherder with 12 char revisions

Status: RESOLVED FIXED

(Taskcluster :: General, defect)

Description

[Cameron Dawson [:camd]]

We are transitioning to 40 char revisions.  Would you please update mozilla-taskcluster to submit the long revisions?

I put an instrumented build on Treeherder stage which spews exceptions to new relic when a short revision is submitted via our /resultset/ endpoint.

Here's a couple example revisions from Friday:
1205efecce10
13233a347df5


I'm not sure how persistent the new relic errors are, but here's a link to one of them.  You can see in the params the "username" and "revision" values on the right side.
https://rpm.newrelic.com/accounts/677903/applications/5585473/filterable_errors#/show/553eb7-c572a9ef-d750-11e5-a1e3-b82a72d22a14/stack_trace?top_facet=transactionUiName&primary_facet=error.class&barchart=barchart&filters=[{%22key%22%3A%22error.class%22%2C%22value%22%3A%22exceptions%3AException%22%2C%22like%22%3Afalse%2C%22spaName%22%3A%22APM%20Filterable%20Errors%22%2C%22timestamp%22%3A1455917787285%2C%22timeSinceLoad%22%3A7.369%2C%22browserWidth%22%3A1280%2C%22browserHeight%22%3A1442%2C%22referrerUrl%22%3A%22https%3A%2F%2Frpm.newrelic.com%2Faccounts%2F677903%2Fapplications%2F5585473%22%2C%22currentUrl%22%3A%22https%3A%2F%2Frpm.newrelic.com%2Faccounts%2F677903%2Fapplications%2F5585473%2Ffilterable_errors%22%2C%22pageUrl%22%3A%22https%3A%2F%2Frpm.newrelic.com%2Faccounts%2F677903%2Fapplications%2F5585473%2Ffilterable_errors%22%2C%22eventType%22%3A%22PageAction%22%2C%22actionName%22%3A%22ADD_FILTER%22}]&_k=71qiak

Comment 1

[Cameron Dawson [:camd]]

Hey man, this is the bug I was talking about on Friday.

Comment 2

[Greg Arndt [:garndt]]

I don't have access to new relic...is it possible to send me some of the details or a job on treeherder that this happened with?  I would imagine that mozilla-taskcluster would either always be sending it or never be sending short versions....I'm not immediately sure why this is yet as I think we just use what comes from the push log.

Comment 3

[Cameron Dawson [:camd]]

oh, sorry about the new relic access..  I'll see if I can add you to that.  But here are some of the details.

An example of a resultset that has the short revision is: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=8574c5890ef6

Not sure a screenshot of the new relic data has value, but here it is: https://www.dropbox.com/s/5bzmghxw5s6if30/Screenshot%202016-02-22%2009.04.59.png?dl=0

Comment 4

[Cameron Dawson [:camd]]

To be honest, when you call our endpoints, you always get the 12 char revision, because we haven't transitioned to the longer revisions on the front-end.  So that could look pretty misleading (all resultsets look like we ONLY have the 12 char revision).  

But that shouldn't affect anything with this.  This is purely an issue of the revision length at time of creation.  But it sounds like the source you're using may give you a 12 char?

Comment 5

[Cameron Dawson [:camd]]

Oh, it occurred to me that TH used to barf if given a 40 char revision, so we asked submitters to always truncate to 12.  Now we're changing our minds.  -- TH is a fickle beast...  :)  Is it possible jlal did that somewhere in your code?  Just throwing that out, because it maybe it's getting truncated in a non-obvious place?  Just a guess...

Comment 6

[Greg Arndt [:garndt]]

Thanks for the suggestion, I found this:
https://github.com/taskcluster/mozilla-taskcluster/blob/master/src/treeherder/resultset.js#L26

Comment 7

[Cameron Dawson [:camd]]

Oh awesome.  That's an easy change!  :)

Comment 8

[Greg Arndt [:garndt]]

so things are ok to be submitted without that restriction now?

Comment 9

[Cameron Dawson [:camd]]

they are, yeah.  We are submitting to those same APIs with 40 char revisions internally at the moment.

Comment 10

[Greg Arndt [:garndt]]

Attachment: https://github.com/taskcluster/mozilla-taskcluster/pull/49

Comment 11

[Cameron Dawson [:camd]]

Comment on attachment 8722233 [details] [review]
https://github.com/taskcluster/mozilla-taskcluster/pull/49

yep, looks right to me.  Thanks!

Comment 12

[Greg Arndt [:garndt]]

Merged and deployed