newCluster() broken code about go-marathon HOT 4 CLOSED

@mattes agreed! ... we should validate each one ... I'll raise a PR

from go-marathon.

@gambol99 Are you working on this one? Otherwise, I'd like to take a stab.

from go-marathon.

After a bit of investigation, I don't think we need to make any significant changes. Specifically,

1. The parsed Host field is already split on comma, so we're not using the comma-separated list directly. Parsing the marathonURL passed to newCluster still makes sense though since the schema needs to be (and is) verified.
2. Applying url.Parse()/url.ParseRequestURI() on each host doesn't really add a lot of value: The main job of the methods is to parse the schema and host, retrieve authorization data, and do unescaping. We already do the first one on marathonURL and don't really care about the latter two as the contained hosts will eventually be used in a request to Marathon as-is. (Side note: To me it seems that Golang's parsing functions are mainly for parsing URLs structurally, not validation.)

The only thing I'd recommend at this stage is to check each host for emptiness and add tests. WDYT?

from go-marathon.

I put together a PR based on my observations/assumptions pro-actively. Happy to discuss. :)

from go-marathon.