API for rescheduling keepalive pinging

[lgrahl]

For the SaltyRTC use case, we need to schedule the keepalive ping with a specific interval (and perhaps timeout in the future) as requested by the client. That would require an additional API surface.

I'm also wondering if it is a good idea to activate the keepalive ping by default but I don't have a strong opinion on that. Nevertheless, it was surprising to me to see two keepalives being running (since the SaltyRTC server does have its own keepalive implementation) which I didn't expect.

[lgrahl]

(To say that this is out of the scope of this implementation would also be fine by me.)

[aaugustin]

connect and serve (and WebSocketClientProtocol and WebSocketServerProtocol) accept ping_interval and ping_timeout arguments. Isn't that enough to "schedule the keepalive ping with a specific interval (and perhaps timeout in the future) as requested by the client"?

Pings are enabled by default in response to feedback from users who complained that websockets doesn't detect disconnections. I think it's the right choice for users who aren't going to think about pings. (The default matters less for those who are going to make a conscious decision.)

[lgrahl]

The issue is that we do a handshake before the keepalive interval is known.

The default matters less for those who are going to make a conscious decision.

Agree but it was surprising to an existing project. And IIRC it is missing in the changelog so far.

[aaugustin]

It's the first item in the 7.0 changelog after the backwards-incompatible changes:

* websockets sends Ping frames at regular intervals and closes the connection
  if it doesn't receive a matching Pong frame. See
  :class:`~protocol.WebSocketCommonProtocol` for details.

[lgrahl]

Oh, I somehow assumed it would be another warning. Never mind.

[aaugustin]

You could connect with ping_interval = None, then, once you know the parameters:

def handler(ws, path):
    # ...
    # Emulate how websockets configures and starts its ping keepalive task (private APIs).
    ws.ping_interval = ping_interval
    ws.ping_timeout = ping_timeout
    ws.keepalive_ping_task = asyncio_ensure_future(
        ws.keepalive_ping(), loop=ws.loop
    )

These are private APIs but you can probably get away with that for the foreseeable future.

[lgrahl]

That's a bit hacky. :neutral_face:

Don't be shy to say it's out of the scope. Keeping the API surface small is a good thing after all and our current solution (just doing pinging ourselves) works. :slightly_smiling_face:

[aaugustin]

Yes, if you already have a working keepalive ping, you can simply disable the built-in feature and continue using your own.