fix: keep every part of an api request the caller wrote - #311
Open
mkb79 wants to merge 7 commits into
Open
Conversation
The endpoint went to the client as it was typed and the query was built
beside it, so parts of a request never left the machine and parts that
did were not the ones that had been written.
- A query written into the endpoint was erased. `api "library?num_results=5"`
asked for the whole library, because the command always passed
`params={}` and httpx replaces the url query with whatever `params`
says rather than merging the two.
- The same key twice kept only the last, so `-p asins=A -p asins=B`
asked after one book.
- `-p k=a=b`, `-p broken` and `-b '{'` reached the code that cannot take
them and left a traceback and exit 3. `-i vier` was worse: it exited 0
after prefixing every nested line of the answer with `vier`.
- An absolute url was sent as given, signed for the host it named.
- A 200 carrying something other than json became a json string and
exited 0, so a maintenance page read as success.
- The client was built here instead of through the session, so
`--timeout` did nothing and every call gave up after the ten seconds
the library defaults to.
- `-f dict -o FILE` handed a dict to `write_text` and died with a
`TypeError`.
The endpoint is a path now. The marketplace comes from the profile or
`--country-code`, and a url is refused before any credentials are
loaded. A query in the path and one in `--query` add up, repeats
included. The answer has to be json, and json is what stdout carries.
What the command cannot send, it refuses rather than drops: a body on a
GET or a DELETE, a body of `null` (the client passes it to httpx as
`json=None`, which writes an empty body), `NaN` and `Infinity`, an
endpoint that names nothing, a fragment.
`--param` is spelled `--query`, which gives `-p` back to the group it
belongs to, where it is the password. `--header`, `--body-file` and
`--dump-header` are new; the headers that carry the authentication or
describe the body are refused. `--format` takes only json and says it is
on its way out.
The command had no tests. The suite pins the request that goes out and the answers the command gives back: the query from the path and from the options together, repeats and values carrying `=`, percent escapes the path keeps, headers that repeat and headers that are refused, the body from an option, a file and a pipe, and every refusal that happens before a client is built. One case goes through the real `AsyncClient` behind a `MockTransport` and reads the url off the outgoing request. A stand-in client cannot show that httpx replaces the query rather than merging it, which is the mistake that erased the query in the first place.
The README section says that the endpoint is a path, how the two ways of writing a query add up, where the paging headers come from, and what happened to `-p`.
The file leaves the formatter's exclusion list. Layout only: an option decorator puts each argument on its own line, the continuation lines of a help text line up with the arguments around them, and the set of refused headers spreads over the frozenset call.
Each parameter type names what it converts to, through the type argument click's ParamType carries: `ApiPath` a path and its query, the two pair types a name and a value. What stays `Any` is what click hands over as `Any` -- the string a converter is given, and the mapping of options a command receives. Under `mypy --strict` the file is left with the two errors its decorators bring: `run_async` and `timeout_option` are untyped, which makes every command they wrap untyped.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
audible apiis for the Audible API and for json. This narrows it tothat, and keeps every part of a request the caller wrote.
What was wrong
Reproduced against
master:api "library?num_results=5"api library -p asins=A -p asins=Bapi library -p k=a=b/-p broken/-b '{'api library -i viervierapi library -f dict -o out.jsonTypeError, exit 3api https://foreign.host/collectapi library --timeout 60The query was erased because the command always passed
params={}, andhttpx replaces the url query with whatever
paramssays rather thanmerging the two.
What it does now
The endpoint is a path. The marketplace comes from the profile or
--country-code, so it is decided in one place, and a url is refusedbefore any credentials are loaded. A query written into the path and one
given with
--queryadd up, repeats included. The answer has to bejson, and json is what stdout carries.
What the command cannot send it refuses rather than drops: a body on a
GETor aDELETE, a body ofnull,NaNandInfinity, an endpointthat names nothing, a fragment.
New:
--header/-H(repeatable, the same name twice included; theheaders that carry the authentication or describe the body are refused),
--body-file(a file, or-for standard input),--dump-header/-D(the status line and the response headers, written even when the call
failed —
total-countandcontinuation-tokenlive there).--paramis now--query/-qand says it is old, which gives-pbackto the group, where it is the password.
--formattakes onlyjsonandsays it is on its way out;
-f dictprinted a Python literal that nojson reader takes.
Not covered here
The hosts that are not the API, and answers that are not json, move to
audible request, which follows before the release. Nothing is droppedwithout a replacement being there.
Tests
The command had none. 60 now, including one that goes through the real
AsyncClientbehind aMockTransportand reads the url off theoutgoing request — a stand-in client cannot show the httpx behaviour
that erased the query.