tpodlaski / copra

Ansyncronous REST and WebSocket Python clients for the Coinbase Pro virtual currency trading platform.
MIT License
49 stars 15 forks source link

Async vs callback clients #3

Closed charles-cooper closed 5 years ago

charles-cooper commented 5 years ago

Perhaps this is more of a question than an issue, but I'm wondering why the APIs for the rest and websocket clients differ in style? The rest async API seems more composable but perhaps there is some benefit to using autobahn that I am missing.

tpodlaski commented 5 years ago

The styles of the two APIs are definitely different in that the WebSocket client is centered around callbacks while the REST API consists of user-callable, asynchronous methods attached to the client object. This is driven more by the protocols themselves than by any conscientious design choice. Unless I am misunderstanding the title of this issue, it is not "async vs. callback," however, since both clients are asynchronous. I do agree that the REST API is more composable, although I would argue the WebSocket client is as well to the extent it can be.

As I state in the documentation, my goal with the REST client was to faithfully replicate the Coinbase Pro REST API endpoints. As the REST protocol is stateless and connections are relatively short-lived, it was straight forward to create regular, asynchronous methods for making these calls.

Websockets, on the other hand, are stateful and the connections are long lived with bi-directional, asyncronous communication. This necessitates the use of callbacks. Although the callback methods themselves are synchronous, they are called asynchronously by the underlying autobahn code. All of the autobahn code for connecting, subscribing, and listening for incoming messages is asynchronous.

I had originally written the WebSocket client directly on top of asyncio but that required writing and implementing my own WebSocket protocol. When I found out that autobahn supported asyncio in addition to Twisted and also had a WebSocket client right out of the box, I switched to it. My goal wasn't to offer a generic WebSocket client but a WebSocket client specifically for Coinbase Pro so I was able to avoid all of the actual WebSocket protocol programming by using autobahn.

The WebSocket client does do more abstraction and encapsulation than the REST client. This happens mostly in the Channel class. I found that making subscription calls was fairly complicated without wrapping them in a class. As I didn't envison most applications doing a lot of subscribing and unsubscribing during their lifetime, I felt this was a fair tradeoff for not sticking as closely in that regard to the Coinbase Pro WebSocket API.

In terms of composability, I think the REST client appears more composable because although it operates in the asyncio event loop, it is not event driven the way the WebSocket callbacks are.

In practical use, I am finding the composability of both meet my needs. My weapon of choice for Coinbase Pro trading is actually an asynchronous FIX client I wrote but have not (yet?) included here. The FIX client uses the WebSocket client as the basis for a level 3 order book and uses the REST client for actions such as account manipulation that are not available through the FIX protocol as implemented by Coinbase at this time. Actually, the order book uses the REST client as well in order to load the initial order book snapshot.

I am not sure if I was able to answer your question or not but would gladly continue this dialogue if you would like. Any other feedback you have would gladly be accepted as well. Thanks!

charles-cooper commented 5 years ago

@tpodlaski Thanks for the thoughts! I looked at https://websockets.readthedocs.io/en/stable/intro.html and thought that the CBP client could be implemented something like

import websockets as ws

async def subscribe_channel(conn, chan, argz) : 
    await conn.send(get_subscribe_msg(argz))
should_run = True
async def main() : 
    await subscribe_channel()
    my_state = []
    while should_run : 
        payload = await conn.recv()
        for line in payload.splitlines() : 
            msg = json.loads(line)
            # do something with msg, e.g. my_state.append(msg)

This allows one to handle messages and state without using objects or (multiple?) inheritance. I guess the main downside is that state sharing requires the use of asyncio synchronization which leaves one a little at the mercy of the scheduler. However, requiring asyncio synchronization is almost a good thing, since it requires the programmer to be more explicit how they are demuxing channels and sharing state.

As an example, I implemented a class which 'inverts' the control back to async/await style:

import copra.websocket as ws
class EventfulClient(ws.Client) :
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self.event_queue = asyncio.Queue()

    def on_message(self, msg) :
        self.event_queue.put_nowait(msg)

# ..., pseudocode sketch follows
async def user_code(c, mut, state) :
    mut.acquire()
    while True :
        events = await c.event_queue.get()
        # do stuff with events
        if condition_holds(state) : 
            mut.release()

async def wait_for_condition(mut, state) : 
    await mut.acquire()
    # do stuff with state
mut = asyncio.Lock()
state = create_state()
asyncio.run_until_complete(asyncio.gather(
  user_code(client, mut, state),
  wait_for_condition(mut, state)
  ))

Hopefully this demonstrates what I'm talking about even in my coffee-deprived state! If there is any conclusion here, it's that it might be good to expose an async API as well as the event-driven API since it's easier to go from async -> callback/event-driven than the other way. But perhaps it's better to leave that kind of synchronization to user code.

tpodlaski commented 5 years ago

@charles-cooper Yes, that totally makes sense. I now understand what you meant regarding by the difference between async and callback programming, and I agree that the REST client and WebSocket client are different styles. I am sorry that my initial reply was long and off-target.

Armed with that knowledge I can explain why they are different now. Unlike the REST client, the WebSocket client was not built from the ground up. Rather, it was extracted from some order book code I had written when I first started playing with the API. So the idea of inheriting it was built in from the beginning. In addition, it was not originally asynchronous but was written to work in a multi-threaded environment. Managing locks was a real pain in the butt that disappeared as soon I switched over to asyncio, this being the result of encapsulating all of the logic for a specific application of the WebSocket in a subclass.

If the genesis of the WebSocket client had been different I may have ended up implementing the async style instead, but because I was already using the code, I didn't consider alternatives.

I am in agreement with the advantanges you lay out for the async style. Your own code doesn't need to be OO to use it, there isn't any need for inheritance and the code more clearly shows how shared states are handled.

Practically speaking I have not encountered the need for much demuxing given that with the OO/callback approach it's been easy enough to instantiate a new client for different channels if sorting messages becomes complex. In those cases it wasn't much effort to create a new sublass and just override the on_message method with the logic I needed to handle the messages. The drawbacks of this are obvious: the client has to be subclassed and a separate websocket connection opened and managed. I personally have not encountered any problems doing it this way, but admit the approach does have it's limits.

So I guess the advantages of the callback/inheritance style is that it is OO friendly and in most instances does not have to consider shared state.

I agree the async style being lower level is easier to convert to event driven than vice versa. I also agree that it has it place. Offering an async API seems like a good idea, my only thought is that would make the client more complicated than I intended it to be. I suppose I did not guess the user base very well.