1525789 - HG Perfherder links are broken due to recent TH push endpoint change

Status: RESOLVED FIXED

(Developer Services :: Mercurial: hg.mozilla.org, defect, P2)

Description

[Sarah Clements [:sclements]]

Per a conversation in the #Perfherder chat with :erahm, this link is broken. Upon investigating and talking with camd, this appears to be due to a recent change to the Treeherder /push/ endpoint (swapped out resultset for push and made changes to only retrieve a push for a revision tip).

So for example this link is used to validate the revision in the compare view but currently doesn't return any results since its a sub commit whereas previously it did (I checked out a commit on TH from 1/28 before the endpoint change landed to confirm).

camd is on top of it so just filing this in case other people discover it in the meantime.

Comment 1

[Ed Morley [:emorley]]

The change in question occurred as part of bug 1306707, in order to fix bug 1498644.

However I would very much prefer for the place that is generating these URLs to be fixed, instead of that change being reverted, since:

1. Treeherder already limits the number of stored commits per push to 200 [1], so if such a URL referenced a sub-commit outside the 200 range, it would have already been broken prior to this change. Plus we're exploring reducing that 200 limit even further (see bug 1498644 comment 4)

2. Using a sub-commit SHA in the URL is at best misleading, since it suggests Perfherder has finer grained data resolution than at the per-push level

Where are these URLs being generated?

[1] https://github.com/mozilla/treeherder/blob/870b24438eeae9eeeaf33bd4871a485ffd299aa5/treeherder/etl/push_loader.py#L235-L257

Comment 2

[Ed Morley [:emorley]]

Ah I see "Hg" in the bug summary - guessing this is about the hg.mozilla.org pushlog links?

If so, these are generated (and so should be fixed) here:
https://dxr.mozilla.org/hgcustom_version-control-tools/rev/670f60febb867f4c77cd92cde2af5dcab69cdc83/hgext/hgmo/__init__.py#236
https://dxr.mozilla.org/hgcustom_version-control-tools/rev/670f60febb867f4c77cd92cde2af5dcab69cdc83/hghooks/mozhghooks/push_printurls.py#53

Comment 3

[Sarah Clements [:sclements]]

Yes, these are for the Perfherder links generated here in HG - the broken link was found here: https://hg.mozilla.org/mozilla-central/rev/d623e8eff5e3

I was initially thinking we could try to change whether the urls are generated for sub commits, but the push endpoint is used by the Perfherder app in multiple places to validate revisions (for query param validation or for the compareChooser page, to validate a revision entered into the input), so if we don't roll this back/make a change we'd need to remove the validation from Perfherder or find some alternative for revision validation.

Comment 4

[Ed Morley [:emorley]]

I still believe supporting sub-commits is a bug, and not something that should be supported anywhere.

Comment 5

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/4587

Comment 6

[Cameron Dawson [:camd]]

So I merged this fix adding a commit_revision parameter so as to remedy the current situation without compromising the performance improvement of the revision param only searching the tip revision. But that's not to say we shouldn't address this bug as Ed says. I just didn't want to break backward compatibility.

Comment 7

[Kim Moir [:kmoir] ET]

:camd is this fixed? I see that the PR has been merged. I'm not sure how often TH is deployed.

Comment 8

[Cameron Dawson [:camd]]

Kim-- This has definitely been pushed to production. Are you still experiencing errors with it? If so, let's re-open this bug and explain what you're seeing and I'll see what I can do.

Comment 9

[Cameron Dawson [:camd]]

I believe this to be fixed. Please reopen if you find an error with this.