Repository navigation
Expose core API at top-level - #107
Conversation
|
@cgl I think this really helps clean things up. Does this all make sense from your perspective? Getting the top-level API neatly constrained in time for the sprints seems important to me |
|
Also, since I mentioned "in time for the sprints"... something else that might be worth getting up and running by then is.... async with hip.Server(...) as server:
...(I know, right!!!) Like, actually not that challenging from where we are, because...
Alternatively we start by simply being clear / documenting that we don't yet have the async variant supported, though we're looking at it. Alternatively, alternatively, we could drive this the other way around, by wiring up for the async case instead, and then working towards adding sync support. Adapting the codebase for Anyways, I digress. |
cgl
left a comment
There was a problem hiding this comment.
Looks great overall. Nice work cleaning up and organising all 💯
| @@ -1,43 +1,37 @@ | |||
| from ._client import * # Client | |||
There was a problem hiding this comment.
Happy to say goodbye to these import * lines.
| if remaining is None or dur < remaining: | ||
| time.sleep(dur) | ||
| else: | ||
| time.sleep(dur - remaining) |
There was a problem hiding this comment.
with hip.timeout(1):
hip.sleep(1.2)
For this example the current code sleeps about 0.2 seconds before raising.
There was a problem hiding this comment.
Fixed, thanks!
I've had to add a nocover directive because it's a bit fiddly to reliably test, tho.
|
Incidentally, those tests take a lot longer than I'd expect in the GitHub env. They run through in < 0.3s on my local machine. They list upwards of 60 seconds here. Anecdotally, I think GitHub might delay test matrixes with multiple combos. Perhaps we'd have neater faster tests if we only checked Python lower/higher bounds??? (Or eg. test a single python version on PRs, and the whole matrix on |
Only the core API is exposed at the top-level...
ClientBinary,Content,File,Files,Form,HTML,JSON,MultiPart,TextHeadersMethod,RequestStatusCode,ResponseServerStreamsleep,timeoutQueryParams,URLWe also have some semi-private undocumented API...
hip.parsers(HTTPParser)hip.backends(NetworkBackend,NetworkStream)hip.transports(Transport,Connection,ConnectionPool)Some thoughts around this...
timeout/sleep, also we haven't currently implemented their usage in eitherConnectionorServer.hip.HTTPException. Some classes might raise built-in exceptions in some cases. Eg.ValueError. We should have a think though / double check the expectations around possibleclient.request()exception cases.client.stream()streaming API should be public yet. It touches on some subtle points that I'm not sure we've finalised yet. Perhaps we can remove this for now.StreamorContentyet.Client/Serveryet. Constraining the allowable interface there ensures that we've only got public API that can also be properly mirrored in async.* Notes:
hip.parsers.ProtocolError- do we need vs. a built-in?open_connectionin favour of.open()/ context managed usage.