DRV-302: Introduce streaming api #165
Merged
Conversation
This file contains 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
trevorsibanda
requested review from
agourlay,
erickpintor,
evbruno,
marrony and
ryancharris
|
Working on fixing the CI. Will open a new PR shortly for that |
trevorsibanda
force-pushed
the
streaming-api
branch
8 times, most recently
from
980f093 to
21967ae
Compare
agourlay
reviewed
Nov 2, 2020
|
I did a first pass and it looks good so far 👍 |
erickpintor
reviewed
Nov 2, 2020
| Current limitations: | ||
| Python requests module uses HTTP1; hyper is used for HTTP/2 | ||
| """ | ||
| def __init__(self, client, expression, options): |
There was a problem hiding this comment.
Should we use kwargs for options?
faunadb/client.py
Outdated
| @@ -187,6 +189,31 @@ def query(self, expression, timeout_millis=None): | |||
| """ | |||
| return self._execute("POST", "", _wrap(expression), with_txn_time=True, query_timeout_ms=timeout_millis) | |||
|
|
|||
| def stream(self, expression, options=None, on_start=None, on_error=None, on_version=None, on_history=None, blocking=True): | |||
There was a problem hiding this comment.
Maybe use kwargs for options and event handlers?
There was a problem hiding this comment.
I would lean more on not using kwargs since we have a finite and well defined set of possible events and names of event handlers. IMO it also improves readability and documentation.
trevorsibanda
force-pushed
the
streaming-api
branch
2 times, most recently
from
110886e to
d7734a8
Compare
ryancharris
reviewed
Nov 4, 2020
trevorsibanda
force-pushed
the
streaming-api
branch
2 times, most recently
from
8fbd64e to
8f92ca9
Compare
benjumanji
reviewed
Nov 6, 2020
trevorsibanda
force-pushed
the
streaming-api
branch
from
8f92ca9 to
e76f188
Compare
erickpintor
requested changes
Nov 10, 2020
trevorsibanda
force-pushed
the
streaming-api
branch
from
e76f188 to
809911d
Compare
|
LGTM feature wise but my level in Python does not allow me to give a real approval. |
benjumanji
previously approved these changes
Nov 13, 2020
erickpintor
reviewed
Nov 13, 2020
trevorsibanda
force-pushed
the
streaming-api
branch
from
809911d to
4752921
Compare
benjumanji
approved these changes
Nov 17, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Introduces the streaming api for the Python driver.
Notable changes and limitations.
blocking=Falseto the FaunaClient stream method, the event loop will be started in a separate thread.ss -K dst 127.0.0.1 dport = 8443with an active stream.