Dustin J. Mitchell [:dustin] (he/him) Reporter
	Description • 6 years ago

This is just a sanity check, so not blocking anything.

At least, tc-client should barf on invalid rootUrls, but it would be nice for services to do the same at startup.

	Dustin J. Mitchell [:dustin] (he/him) Reporter
	Comment 1 • 6 years ago

Per https://github.com/taskcluster/taskcluster-lib-urls/pull/15#issuecomment-424074381 Pete's going to work on this.

I suspect just parsing the URL and asserting that the hostname is set and the path is empty is enough.  In my experience working with rootUrls so far, the most common error is omitting the rootUrl, rather than any more subtle error.

	Pete Moore [:pmoore][:pete]
	Comment 2 • 6 years ago

A first stab at this here for the go library:

https://github.com/taskcluster/taskcluster-lib-urls/blob/3b9cc425cdf19a0e7312d9dcd81902137354e6dd/tcurls.go

	Dustin J. Mitchell [:dustin] (he/him) Reporter
	Comment 3 • 6 years ago

I like it!  It looks like this checks for validity when constructing a RootUrl object, meaning that all of the URL-generation functions don't return an error.

Note that the Python library doesn't have this concept of a RootUrl object (it probably should), but does have some "raw" functions that take a string rootUrl as their first argument.  The JS library has all of the above.  Perhaps we should converge on "all of the above" for all three languages?  So (mashing up language syntaxes):

  libUrls.api("https://tc.mycompany.com", ..) -> (string, error)
  const urls = libUrls.withRoot("https://tc.mycompany.com") -> (obj, error)
    urls.api(..) -> string

In the JS version, the first ("raw function") is implemented in terms of the second.

(by the way, since it's easy enough and just a testing utility returning a static string, it'd be great to add testRootUrl to the Python / Go clients at the same time)