1046225 - If the repo specified in the URL does not exist, check if the case was incorrect

Status: RESOLVED WONTFIX

(Tree Management :: Treeherder, defect, P3)

Description

[Ed Morley [:emorley]]

This URL doesn't work:
https://treeherder.mozilla.org/ui/#/jobs?repo=Try

The correct URL is:
https://treeherder.mozilla.org/ui/#/jobs?repo=try

TBPL used uppercase, so I imagine:
a) People's muscle memory will cause them to use the wrong case
b) When performing search and replace (eg in bug 1043880) the resultant URLs don't work. This becomes even harder to spot for bugs like bug 1043880, where it's not even clear what case will be passed in.

Could we redirect to the correct repo case? (or at least redirect to lowercase, presuming all valid names happen to be lower-case at the moment?)

Comment 1

[Hal Wine [:hwine] use NI!]

in general, repo names are case-sensitive on their respective servers. Since we have developers on windows & macs, we _should_ never have two repositories which differ only by case. So I think the retry may be reasonable for now.

We do have cases where the same 'repo base name' is used in two different directory trees. That has tripped us up in the past with attempts to "simplify" URLs. (See, e.g. {build,qa}/puppet.) While there are no FF product code repos with that issue currently, it's not on our list of things to verify about new repo names.

If there are designed in assumptions about uniqueness of repo names, it would be good to update the repo creation procedure to include those. see https://wiki.mozilla.org/ReleaseEngineering/RepositoryCreationRequest#Behind_the_Scenes for similar issues.

Comment 2

[Ed Morley [:emorley]]

12:36 <froydnj> hm, treeherder doesn't want to disply my try jobs
12:39 <bbouvier> froydnj: maybe just need to wait a few minutes... for me, they usually show up a few minutes after they show up on try
12:42 <edmorley> froydnj: could you paste the url?
12:48 <froydnj> edmorley: https://treeherder.mozilla.org/ui/#/jobs?repo=Try&revision=117245a61c89
12:48 <froydnj> edmorley: https://treeherder.mozilla.org/ui/#/jobs?repo=Try&revision=36005ce6d43c
12:48 <froydnj> jobs showing up on tbpl just fine
12:48 <edmorley> froydnj: try repo=try (lowercase)
12:48 <edmorley> froydnj: presume these came from the email links?
12:49 <froydnj> edmorley: aye
12:49 <froydnj> silly computers, DWIM!
12:50 <edmorley> froydnj: so the repo name used in buildbotcustom's try mailer (that TBPL is happy with) isn't liked by treeherder. Bug 1046225 is on file for making treeherder more tolerant

Comment 3

[Ed Morley [:emorley]]

Ok, given that it now seems that the uppercase URLs don't originate from buildbot, but are just hardcoded in a few places (like the try mailer), I think this isn't a blocker. I've attached a follow-up to bug 1043880 that will fix the particular case in comment 2.

Comment 4

[Jeffrey Godwyll [:jeffgodwyll] (away until Dec 15th - use "needinfo")]

I'm interested in working on this. What's a good first place to look? In mozilla-central[i have a local copy running]? This page[ https://wiki.mozilla.org/Auto-tools/Projects/Treeherder] suggests a couple of places to start. Will I have to clone any of those? Thanks.

Comment 5

[Ed Morley [:emorley]]

(In reply to Jeffrey Godwyll [:jeffgodwyll] from comment #4)
> I'm interested in working on this. What's a good first place to look? In
> mozilla-central[i have a local copy running]? This page[
> https://wiki.mozilla.org/Auto-tools/Projects/Treeherder] suggests a couple
> of places to start. Will I have to clone any of those? Thanks.

Hi Jeffrey, that's great! Mauro is the mentor for this bug, though he's away for the week, so I'll do my best to help in the meantime.

For this bug, you won't be needing to use mozilla-central - instead it will require changes to the treeherder-ui Git repo, at:
https://github.com/mozilla/treeherder-ui/

To see the problem this bug is trying to fix:
1) Open a new tab
2) Open the webconsole (ctrl+shift+k or use the web developer menu)
3) With the console open, navigate to https://treeherder.mozilla.org/ui/#/jobs?repo=Try
4) Note in the console you see the warning:
"ThRepositoryModel" "'Try' not found in repos list."

This comes from:
https://github.com/mozilla/treeherder-ui/blob/2883b287399d0ccc8f56a42c54f859815912d31d/webapp/app/js/models/repository.js#L29

In this case, we want to try to look again but using lowercase - and if successful when doing this, update the current repo to the correct name (so the URL updates etc).

Comment 6

[Ed Morley [:emorley]]

To test changes to the UI:
1) Git clone the treeherder-ui
2) Copy webapp/app/js/config/sample.local.conf.js to webapp/app/js/config/local.conf.js
3) Install node (http://nodejs.org/download/)
4) From the source directory created at step #4:
cd webapp
./scripts/web-server.js
5) Navigate to http://localhost:8000/app/index.html#/jobs?repo=Try (the non-typoed URL is http://localhost:8000/app/index.html#/jobs?repo=try)

Comment 7

[Jeffrey Godwyll [:jeffgodwyll] (away until Dec 15th - use "needinfo")]

I'd like to be assigned this. I've replicated all steps.

Quick query: In comment 0, Ed, you made mention of redirecting to lowercased repo names. I want to try this approach, but will first want to know how repositories are named. Is there a list somewhere, are there any conventions?

Thanks.

Comment 8

[Jeffrey Godwyll [:jeffgodwyll] (away until Dec 15th - use "needinfo")]

And from where are we reading the list of repos from. Is it from here (https://github.com/mozilla/treeherder-ui/blob/master/webapp/app/resources/menu.json)?

Comment 9

[Ed Morley [:emorley]]

Unfortunately it seems like menu.json is badly named - it is just sample data as far as I can tell. I've filed bug 1062211 to rename it to something more useful.

The list of repos comes from this API call:
https://treeherder.mozilla.org/api/repository/

See the "name" property.

Let me know if there's anything else I can help with :-)

Comment 10

[Jeffrey Godwyll [:jeffgodwyll] (away until Dec 15th - use "needinfo")]

Attachment: Lower cased repo names

Comment 11

[Jeffrey Godwyll [:jeffgodwyll] (away until Dec 15th - use "needinfo")]

Aaaargh, but all the repositories there are lowercased. No idea where "Try" came from. Will appreciate some help as to where the "Try" is being generated. I've submitted a pull request though. I hope I get it right first time.

Comment 12

[Ed Morley [:emorley]]

Comment on attachment 8484712 [details] [review]
Lower cased repo names

(In reply to Jeffrey Godwyll [:jeffgodwyll] from comment #11)
> Aaaargh, but all the repositories there are lowercased. No idea where "Try"
> came from. Will appreciate some help as to where the "Try" is being
> generated. I've submitted a pull request though. I hope I get it right first
> time.

All of the current repo names are lowercase, the problem is that TBPL (the predecessor to treeherder) used mixed case. As such, people will be used to typing alternative repo names, and the various places that get switched over may link to the wrong name (see comment 0).

Thank you for the pull request - unfortunately that makes the param case insensitive, rather than keeping it case-sensitive, but have a case-insensitive fallback.

(Cancelling the needinfo, since the review request is a superset of it, so only one needs to be set at once :-))

Comment 13

[Ed Morley [:emorley]]

Since this bug was filed, the messaging for an unrecognised repo name has been improved:
https://github.com/mozilla/treeherder-ui/commit/08cc4f1a6603b5fbf353091d6045166144f732e5

Given that it's now clearer the repo name is wrong, and now that people have been using treeherder more (so comment 0 #a is less of an issue) and that it now seems that the mixed case repo names were a TBPL-only ism (and so hardcoded rather than from buildbot) - it seems like this bug may not be worth implementing now.

If you're interested in another bug, how about bug 1053762 or bug 1046229 ? :-)