Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable keep-alive pings with ping_interval=None #343

Closed
iyakushev opened this issue Jul 15, 2022 · 1 comment
Closed

Disable keep-alive pings with ping_interval=None #343

iyakushev opened this issue Jul 15, 2022 · 1 comment
Labels
type: documentation An issue or pull request for improving or updating the documentation

Comments

@iyakushev
Copy link

While testing the WebSocketTransport to run subscriptions I've started to get random errors. Turns out it was keep-alive ping that server ignored:

DEBUG:websockets.client:% sending keepalive ping
DEBUG:websockets.client:> PING 15 b4 8b 07 [binary, 4 bytes]
DEBUG:websockets.client:! failing connection with code 1006
DEBUG:websockets.client:= connection is CLOSED

Fix
My initial thought was to set ping_interval=None and that would stop connection from dropping (passing it through to websockets args). However, this changed nothing. And after a little bit of investigation I tried to pass connect_args={"ping_interval": None} to influence websockets directly and it worked like a charm.

Expected behavior
I believe ping_interval=None should change websockets ping_interval argument. If I'm wrong than this could be stated somewhere in the docs or an argument's docstring. Thanks in advance!

To reproduce
You may query this graph.

from gql import gql, client
from gql.transport.websocket import WebsocketTransport
import asyncio
import logging

TARGET_GRAPH = "wss://api.thegraph.com/subgraphs/name/uniswap/uniswap-v3"
logging.basicConfig(level=logging.DEBUG)

WS = WebsocketsTransport(
   TARGET_GRAPH,
   ping_interval=None,
)
CLIENT = Client(transport=WS, fetch_schema_from_transport=False)

TESTQ = gql("""
subscription test {
 pool(id: "0x8ad599c3a0ff1de082011efddc58f1908eb6e6d8") {
   token0 { symbol decimals }
   token1 { symbol decimals }
   feeTier
   sqrtPrice
   liquidity
 }
}
""")

if __name__ == "__main__":
   async def foo():
       async with client as session:
           async for result in session.subscribe(FACTORY_Q):  # type: ignore
               print(result)

   asyncio.run(foo())
@leszekhanusz
Copy link
Collaborator

I admit that this is a bit confusing but there are actually two different kind of pings at different levels of the protocol.

You have pings at the websockets protocol level, defined in RFC 6455, which can be disabled with connect_args={"ping_interval": None} as you found out.

And you also have pings in the graphql-ws protocol, which are by default disabled and can be enabled with only the ping_interval argument.

I agree that the documentation about this could be clearer. I'll add something about this in the docs.

@leszekhanusz leszekhanusz added the type: documentation An issue or pull request for improving or updating the documentation label Jul 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation An issue or pull request for improving or updating the documentation
Projects
None yet
Development

No branches or pull requests

2 participants