1313274 - self-serve should only accept 40 character SHA revisions

Status: RESOLVED WONTFIX

(Release Engineering :: General, defect, P3)

Description

[Nick Thomas [:nthomas] (UTC+12)]

Treeherder is going to require 40 character SHA revisions, and self-serve currently allows 12 characters. In the last week there's one example where [e-mail] used self-serve to make a request for 75c306c21c0f on fx-team. I think jobs won't appear on TH if we don't require 40 chars on the API. CC armen since mozci-bot will need to handle this too.

Comment 1

[Nick Thomas [:nthomas] (UTC+12)]

https://dxr.mozilla.org/build-central/source/tools/lib/python/kickoff/__init__.py#27 is one way to do this.

Comment 2

[Armen [:armenzg]]

I can easily fix this. Let me know when you would like to switch to it.

Comment 3

[Nick Thomas [:nthomas] (UTC+12)]

Pretty sure you can submit 40 character revisions already, so go ahead stop truncating if you are already. Mostly this is about making sure insist on 40 char, or allow 12 and convert to 40.

Comment 4

[Armen [:armenzg]]

Could you please help me find instances of it?
Over a year ago we landed code to only use 40 chars and I'm surprised to see this. I'm not saying that it is not happening I just can't find where to fix it.

At the moment I can't find any requests of 12 chars:
https://secure.pub.build.mozilla.org/buildapi/self-serve/jobs

Comment 5

[Nick Thomas [:nthomas] (UTC+12)]

Attachment: 12 char mozci from the last week

Might be someone using the command line rather than your automation ?

Comment 6

[Nick Thomas [:nthomas] (UTC+12)]

The timestamp is PDT in that.

Comment 7

[Armen [:armenzg]]

Attachment: [mozhginfo] - Do not allow the Push object to use changesets that are not 40 chars in lenght. r=gps

This change makes pulse_actions to receive exceptions if anyone tries to create pushes that are not-40-chars in length.

This forces the caller to fix things on their side.

Comment 8

[Armen [:armenzg]]

Attachment: [mozci] - Force revisions to have 40-chars

If on trigger_job() we use a revision that is not 40-chars, we try to determine the 40-char version of it via mozhginfo.

I just deployed this to Heroku as we're seeing a bunch of errors unhandled (thanks to the previous attachment).

If there are any other callers that are using 12-chars we will find out after the current cloud of alerts clears up.

nthomas: unless there's anymore improper callers, we should not see mozci-bot request jobs with 12-char revisions.

Comment 9

[Nick Thomas [:nthomas] (UTC+12)]

Thanks Armen. Will let you know if I find anything new.

Comment 10

[Ed Morley [:emorley]]

There are also humans submitting with 12 character SHAs via the web UI.

Once tooling is converted to using 40 character SHAs, could we just forbid <40 chars too? (Checking that an appropriate error message is shown to users of the web UI)

Comment 11

[Phil Ringnalda (:philor)]

Is there a plan to have Treeherder UI stop using 12-char in the most prominent place that it does anything with revisions, on the left side of every push, or to make it possible to copy the 40-char somewhere? The reason I submit 12-char to self-serve all the time is because of Copy Link Text on those Treeherder links. Changing to getting it from the self-serve URL instead merely makes it slightly annoying (context menu > URL partial selection) for the cases where I've opened self-serve for the push I'm triggering on, but for the cases where the scheduler raced and so there were no buildbot jobs at all triggered so self-serve for the push can and so I have to open self-serve for a different push, having to first open hg.m.o for the revision, copy from the URL there, close, open, paste, will not make me happy.

Comment 12

[Ed Morley [:emorley]]

Good point Phil, I've filed bug 1340265 for making it easier to copy the full SHA from Treeherder.

Regarding the builds that were triggered using the 12 char SHA - buildbot in fact already converts these into the full 40 characters, but outputs as `sourcestamp` and/or `got_revision`, eg:

    {
      "id": 118114302, 
      ...
      "properties": {
        "branch": "mozilla-central", 
        "buildername": "WINNT 5.2 mozilla-central pgo-build", 
        ...
        "got_revision": "70debab476881121ddfe92fc60c80cfae0a63bc7", 
        "revision": "70debab47688", 
        "sourcestamp": "70debab476881121ddfe92fc60c80cfae0a63bc7", 
        ...
      }, 
      "reason": "Self-serve: Requested by [e-mail]", 
      ...
    }, 

I was thinking Treeherder could just fall back to these other properties, however:
(a) I'm pretty sure they don't exist in builds-{pending,running}
(b) the tests that spawn from that build re-use the short SHA, not the long SHA, and don't set `got_revision` or `sourcestamp` themselves, so we'd still need changes to buildbotcustom/configs

At this point it seems like the easiest way forwards is to either:
1) fix the "easier to copy 40 char SHA from Treeherder" bug (and then optionally ban <40chars in self-serve)
2) try the "convert in self-serve" approach (comment 1)

It might be easier to do (1) given how problematic making changes to self-serve was in bug 1306718.

Comment 13

[Ed Morley [:emorley]]

Bug 1340265 is going with (1), so we should just ban != 40 char revisions instead of the conversion approach.

Comment 14

[Ed Morley [:emorley]]

Self-serve no longer exists.