Typo in transport reference

[lukasolson]

The path parameter is listed twice. I'm pretty sure it's a typo.

[spalger]

nope, path is also the query string params. :P

[jmm]

@spalger Would you mind expanding on your previous comment? Without having noticed this issue, I also interpreted that as a mistake like @lukasolson and in #334 deleted it and specifically included this in the source docs I added for Transport.request() path:

Do not include query string.

I just looked through the code again and it does not seem to me that Transport.request() is really designed to accommodate query string parameters in the path param in any format, though sometimes it sort of works out. Consider this example:

var host = {
  host: "localhost",
  port: 5000,
  path: "/status",
};

var request = {
  method: "GET",
  path: "/fail?request=r",
};

// No query for host. Seems to work out due to the way host & request params are
// concatenated.
new elasticsearch.Transport({
  host: host,
  log: ['trace'],
})
.request(request);

// Query for host. Constructs broken URL.
host.query = "host=h";
new elasticsearch.Transport({
  host: host,
  log: ['trace'],
})
.request(request);

Using the HTTP connector, at the point the second request hands off to http.request(), path is /status/fail?request=r?host=h.

[spalger]

My bad @jmm, I was joking above. Since you fixed it this is closed. Sorry for the confusion

[jmm]

@spalger D'oh! That went over my head because when I was working on #330 it was actually tricky figuring out the inconsistencies in I/O accepted by various modules and described in the documentation. (For example, http.request() requires query string to be passed in its path param, I believe.) For that matter, perhaps it'd make sense to throw if path contains ?, since it's not designed to accept it and can produce inconsistent results (appear to work in some cases and fail in others)?

On a related note, in transports source I documented query as accepting string|object -- is that legit public API you want to document in the asciidoc? Either way (string|object or just object) it should be consistent in both places.

[spalger]

Yeah, I imagine that string|object is a fair contract to document.

These arguments were originally (and naively) designed to map to the hostname, pathname, and query parameters of url.format(), but unfortunately my desire for brevity simply leads to confusion.

[spalger]

Added in 0d25167

[jmm]

@spalger Thanks!

These arguments were originally (and naively) designed to map to the hostname, pathname, and query parameters of url.format(), but unfortunately my desire for brevity simply leads to confusion.

Perhaps the longer names would've been advantageous (you'd know better than I), but bringing the docs more up to date will hopefully make it quite a bit easier to get up to speed with what the param names represent.