1042737 - Make URL scheme shorter & more consistent

Status: RESOLVED INCOMPLETE

(Tree Management :: Treeherder, defect, P4)

Description

[Ed Morley [:emorley]]

This is one of these bugs that isn't critically important - but will be hard to fix later on, once treeherder becomes more widely used & links are pasted everywhere.

URLs are currently of form...

Repo overview:
https://treeherder.mozilla.org/ui/#/jobs?repo=mozilla-central

Single push:
https://treeherder.mozilla.org/ui/#/jobs?repo=mozilla-central&revision=717cd9d89a80

Filtering by pusher:
https://treeherder.mozilla.org/ui/#/jobs?repo=try&author=[e-mail]

I know /ui/ has been used to differentiate from /api/ however it does seem a bit odd to see it in the URL. Also, we're mixing and matching param styles (with the "#/jobs" vs "repo=try&author=[e-mail]") - though I'm presuming some of this might be a limitation of Angular.

Would something like this:
https://treeherder.mozilla.org/jobs/mozilla-central
https://treeherder.mozilla.org/jobs/mozilla-central/rev/717cd9d89a80
https://treeherder.mozilla.org/jobs/try/author/[e-mail]
(though when we start adding the params for filter etc - bug 1033382, we'll need to add "?filter=foo" anyway)

Or:
https://treeherder.mozilla.org/jobs?repo=mozilla-central
https://treeherder.mozilla.org/jobs?repo=mozilla-central&rev=717cd9d89a80
https://treeherder.mozilla.org/jobs?repo=try&author=[e-mail]

Or:
https://treeherder.mozilla.org/jobs#repo=mozilla-central
https://treeherder.mozilla.org/jobs#repo=mozilla-central&rev=717cd9d89a80
https://treeherder.mozilla.org/jobs#repo=try&author=[e-mail]

...be possible?

Comment 1

[Ed Morley [:emorley]]

(In reply to Ed Morley [:edmorley] from comment #0)
> Would something like this:
> https://treeherder.mozilla.org/jobs/mozilla-central
> https://treeherder.mozilla.org/jobs/mozilla-central/rev/717cd9d89a80
> https://treeherder.mozilla.org/jobs/try/author/[e-mail]
> (though when we start adding the params for filter etc - bug 1033382, we'll
> need to add "?filter=foo" anyway)

Another slight variation on this one:

https://treeherder.mozilla.org/jobs/mozilla-central
https://treeherder.mozilla.org/jobs/mozilla-central/717cd9d89a80
https://treeherder.mozilla.org/jobs/try?author=[e-mail]

Which would then allow for:
https://treeherder.mozilla.org/jobs/try?author=[e-mail]&filter=foo
https://treeherder.mozilla.org/jobs/mozilla-central/717cd9d89a80?filter=foo
etc

Comment 2

[Avi Halachmi (:avih)]

Not sure if this bug covers it, but specific views, especially of graph[s] on a specific platform for a specific revision (or some specific dates range) should also have permalinks.

Such permalinks for graphs views have proven very useful when discussing regressions on IRC or bugzilla.

Comment 3

[Ed Morley [:emorley]]

(In reply to Avi Halachmi (:avih) from comment #2)
> Not sure if this bug covers it, but specific views, especially of graph[s]
> on a specific platform for a specific revision (or some specific dates
> range) should also have permalinks.
> 
> Such permalinks for graphs views have proven very useful when discussing
> regressions on IRC or bugzilla.

Treeherder doesn't currently handle any performance graphing - but I believe in the future the plan is to move datazilla into treeherder under the treeherder-plugin/addon architecture, so I guess it makes sense to thing how they will factor into the URL scheme (eg: https://treeherder.mozilla.org/perf/mozilla-central ?)

Comment 4

[Ed Morley [:emorley]]

From dev.platform:

On 25/07/2014 04:37, Nicholas Nethercote wrote:
> Another comment: compare the following TBPL and treeherder URLs:
> 
> - https://tbpl.mozilla.org/?tree=Mozilla-Inbound
> - https://treeherder.mozilla.org/ui/#/jobs?repo=mozilla-inbound
> 
> Does the 'ui/#/jobs' bit add value?

To add some context here - the /jobs is because in the future treeherder will have additional views such as things like "timeline" (failures across time for all trees to help spot infra issues etc) and "machine" (to detect bad machines) as well as any other goodness that's now possible with the improved data model of treeherder vs TBPL.

Comment 5

[[e-mail]]

I take it the /#/ part exists because the appropriate component hasn't been updated to use the HTML5 history API yet?

Comment 6

[Ed Morley [:emorley]]

http://blog.releaseboard.com/2013/12/angular-js-gotcha-2-html5-mode.html

"Angular-route has Hashbang mode and HTML5 mode. Hashbang mode is the default mode and results in ugly urls: app.releaseboard.com/#!/releases. HTML5 mode rewrites your urls to look like standard urls: app.releaseboard.com/releases."

Of use:
http://scotch.io/quick-tips/js/angular/pretty-urls-in-angularjs-removing-the-hashtag
https://github.com/angular-ui/ui-router/wiki/Frequently-Asked-Questions#how-to-configure-your-server-to-work-with-html5mode

Comment 7

[Nicholas Nethercote [inactive]]

> https://treeherder.mozilla.org/jobs/mozilla-central

That's nice! I like the use of '/' instead of '?' and '#', and the lack of 'repo='.

And yes, '?' will come back when adding filters, but at least the default case looks cleaner.

Comment 8

[:Paolo Amadini]

(In reply to Nicholas Nethercote [:njn] from comment #7)
> > https://treeherder.mozilla.org/jobs/mozilla-central

Even better:

https://tree.mozilla.org/jobs/mozilla-central

Note that <http://tree.mozilla.org/> already exists and just redirects to <https://tbpl.mozilla.org/>. Would be great for it to become the canonical URL.

Comment 9

[:Paolo Amadini]

(In reply to :Paolo Amadini from comment #8)
> https://tree.mozilla.org/jobs/mozilla-central

I've been chatting with Ed on IRC and I suggested we could make "tree.mozilla.org" a redirect to the current experimental instance, even before the new URL structure is implemented in Treeherder. This way, the URLs that are sent out automatically on pushes can already be in the final, envisioned format, and pasted into bugs as "official" links.

To do this, a consistent URL scheme for Treeherder should be defined first. For example:

https://tree.mozilla.org/jobs/try/dc7738b44ecb

In this case, the regular expression for the redirect would yield:

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=dc7738b44ecb

In the future, the first URL could just be the canonical one to which Treeherder responds directly.

Comment 10

[Ed Morley [:emorley]]

Whilst it's strongly related (and both will likely need apache redirects), I'm moving the tree.m.o vs treeherder.m.o discussion out to bug 1047387 - since this bug is about the path/query/fragment components rather than the hostname.

Comment 11

[Ed Morley [:emorley]]

Attachment: WIP to enable Angular HTML5 mode

This patch enables HTML5 mode & kinda of works when tested locally, as long as you run webserver.js from the webapp/app/ directory, rather than the webapp/ that the installation instructions state (http://treeherder-ui.readthedocs.org/en/latest/installation.html).

If you choose not to run it from webapp/app/ then index.html requires the addition of:
<base href="/app/">

However that would break production (since web-server.js is only a quick hack to test locally, and production is served with the webapp/app/ directory mapped to treeherder.m.o/ui/ so the href base would be wrong).

Ideally we'd change production so that treeherder.m.o/ maps to webapp/app/ directly, which would/should make this patch work as is (once the necessary apache redirects are set up [1]), as well as clean up the URI scheme further, per previous comments.

The only issue with this patch when testing locally, is that web-server.js doesn't support redirects, so we can't do the steps at [1] and so whilst you can navigate through the app using the html5 history api just fine, if you refresh the page, or try a direct link using the URL in the location bar, you'll get a 404.

To solve this (when testing locally), we could either switch to something more advanced than web-server.js (eg Express) or disable html5mode using local.conf.js.

"Nothing's ever simple..."  sigh.

[1] https://github.com/angular-ui/ui-router/wiki/Frequently-Asked-Questions#how-to-configure-your-server-to-work-with-html5mode

Comment 12

[Ed Morley [:emorley]]

tl;dr:
1) If the rewrites were adjusted so that production served webapp/app/ out of treeherder.m.o/ rather than treeherder.m.o/app/ (which is desirable anyway)
2) The rewrites at the URL in comment 11 are set up.
3) We add a toggle in local.conf.js to the attached patch, so people testing locally using web-server.js can switch back to angular non-html5 mode for the edge cases if/when needed.

...then this should be doable.

However given the issues with local testing of this, it's hard to tell for sure whether things will when deployed (guessing we could use dev to play around?)

Comment 13

[Ed Morley [:emorley]]

(In reply to Ed Morley [:edmorley] from comment #12)
> tl;dr:
> 1) If the rewrites were adjusted so that production served webapp/app/ out
> of treeherder.m.o/ rather than treeherder.m.o/app/ 

I meant:
"rather than treeherder.m.o/ui/"

(Sorry spam)

Comment 14

[Ed Morley [:emorley]]

(In reply to Ed Morley [:edmorley] from comment #12)
> tl;dr:
> 1) If the rewrites were adjusted so that production served webapp/app/ out
> of treeherder.m.o/ rather than treeherder.m.o/ui/ (which is desirable
> anyway)

Filed bug 1063411 for doing that.

Comment 15

[Ed Morley [:emorley]]

Raising priority since I think we should try and tackle this before we start linking to treeherder from too many places.

Comment 16

[Ed Morley [:emorley]]

(Reducing the number of P1 bugs at any one time, so there are a half dozen or so top issues on which efforts can be focused)

Comment 17

[Florian Quèze [:florian]]

(In reply to Ed Morley [:edmorley] from comment #4)
> From dev.platform:
> 
> On 25/07/2014 04:37, Nicholas Nethercote wrote:
> > Another comment: compare the following TBPL and treeherder URLs:
> > 
> > - https://tbpl.mozilla.org/?tree=Mozilla-Inbound
> > - https://treeherder.mozilla.org/ui/#/jobs?repo=mozilla-inbound
> > 
> > Does the 'ui/#/jobs' bit add value?
> 
> To add some context here - the /jobs is because in the future treeherder
> will have additional views such as things like "timeline" (failures across
> time for all trees to help spot infra issues etc) and "machine" (to detect
> bad machines) as well as any other goodness that's now possible with the
> improved data model of treeherder vs TBPL.

Couldn't "/jobs" still be omitted, and "timeline" or "machines" be used instead of the tree name for these future enhancements?

Comment 18

[Ed Morley [:emorley]]

(In reply to Florian Quèze [:florian] [:flo] (Away until March 10) from comment #17)
> Couldn't "/jobs" still be omitted, and "timeline" or "machines" be used
> instead of the tree name for these future enhancements?

It could, which would result in eg:
https://treeherder.mozilla.org/?repo=mozilla-inbound
https://treeherder.mozilla.org/?repo=mozilla-central&revision=foo
https://treeherder.mozilla.org/machines?foo=bar

Though if we were to go even further:
https://treeherder.mozilla.org/mozilla-inbound/
https://treeherder.mozilla.org/mozilla-central/foo/
https://treeherder.mozilla.org/machines/bar/
...it starts to make me uncomfortable mixing and matching repo names with different views.

Open to ideas...

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Were there any plans to revisit this, Ed? At the very least to get rid of the "#/"?

(Just as a side note, I think you now enable html5 mode a little differently with newer Angular. Need to also set requireBase to "/" or something to get things working.)

Comment 20

[Ed Morley [:emorley]]

To get this working would mean:
1) Bikeshedding over new URL scheme
2) Adding custom URL re-write logic to `CustomWhiteNoise` (since we don't have an .htaccess on Heroku)
3) Adding support to web-server.js

Maybe still worth doing, but needs consideration to check it's worth the effort at this point :-)

Comment 21

[Ed Morley [:emorley]]

Given: 

> To get this working would mean:
> 1) Bikeshedding over new URL scheme
> 2) Adding custom URL re-write logic to `CustomWhiteNoise` (since we don't
> have an .htaccess on Heroku)
> 3) Adding support to web-server.js

4) Supporting old URLs in a backwards-compat manner.

...I don't think this is worth it right now.