Allowed passing websockets connection kwargs #914
Conversation
|
In case this PR needs a test case then: import pytest
from websockets.exceptions import ConnectionClosed
from web3 import Web3
w3 = Web3(Web3.WebsocketProvider(max_size=1))
with pytest.raises(ConnectionClosed):
w3.eth.getBlock(0)However, I'm not sure if web3 should testing functionality that should be tested by |
carver
left a comment
carver
left a comment
There was a problem hiding this comment.
I like having the example case you commented with. It provides an example usage, and makes sure the API doesn't change without an explicit test change.
web3/providers/websocket.py
Outdated
| _loop = None | ||
|
|
||
| def __init__(self, endpoint_uri=None): | ||
| def __init__(self, endpoint_uri=None, **kwargs): |
There was a problem hiding this comment.
I'm not wild about letting the websockets connection parameters dictate the whole keyword argument namespace here. How about adding a single dict variable, like connection_params={'max_size': 1}?
That way there is no chance of a future namespace conflict or confusing mismatch. (we already have one mismatch: a value is called uri in websockets, but endpoint_uri here).
There was a problem hiding this comment.
I chose the name websocket_kwargs instead, to be consistent with request_kwargs in HttpProvider.
web3/providers/websocket.py
Outdated
| class PersistentWebSocket: | ||
|
|
||
| def __init__(self, endpoint_uri, loop): | ||
| def __init__(self, endpoint_uri, loop, **kwargs): |
There was a problem hiding this comment.
Same thing here with the kwargs.
web3/providers/websocket.py
Outdated
| async def __aenter__(self): | ||
| if self.ws is None: | ||
| self.ws = await websockets.connect(uri=self.endpoint_uri, loop=self.loop) | ||
| self.ws = await websockets.connect(uri=self.endpoint_uri, loop=self.loop, **self.kwargs) |
There was a problem hiding this comment.
Maybe guard against people passing in loop or uri as one of the connection_params here.
voith
force-pushed
the
allow-passing-websocket-kwargs
branch
from
02ccadc to
b675e5c
Compare
voith
force-pushed
the
allow-passing-websocket-kwargs
branch
from
b675e5c to
ca4a336
Compare
2 participants
What was wrong?
The current implementation of
WebsocketProviderdid not allow configuring extra configurations that awebsocketsconnection takesfixes #913
How was it fixed?
Allowed passing websockets connection kwargs
Cute Animal Picture