HG Perfherder links are broken due to recent TH push endpoint change
Categories
(Developer Services :: Mercurial: hg.mozilla.org, defect, P2)
Tracking
(Not tracked)
People
(Reporter: sclements, Assigned: camd)
References
Details
Attachments
(1 file)
(deleted),
text/x-github-pull-request
|
Details |
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.
Reporter | ||
Updated•6 years ago
|
Comment 1•6 years ago
|
||
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:
-
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)
-
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?
Comment 2•6 years ago
|
||
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
Reporter | ||
Comment 3•6 years ago
|
||
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•6 years ago
|
||
I still believe supporting sub-commits is a bug, and not something that should be supported anywhere.
Comment 5•6 years ago
|
||
Assignee | ||
Comment 6•6 years ago
|
||
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.
Assignee | ||
Updated•6 years ago
|
Comment 7•6 years ago
|
||
:camd is this fixed? I see that the PR has been merged. I'm not sure how often TH is deployed.
Assignee | ||
Comment 8•6 years ago
|
||
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.
Assignee | ||
Comment 9•6 years ago
|
||
I believe this to be fixed. Please reopen if you find an error with this.
Description
•