-
Notifications
You must be signed in to change notification settings - Fork 21
Factored WebSocket handling out of OrderBook to allow for easy creation of custom order book implementations #3
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
Conversation
Codecov Report
@@ Coverage Diff @@ ## master #3 +/- ## ========================================= - Coverage 97.26% 96.2% -1.06% ========================================= Files 4 5 +1 Lines 475 501 +26 ========================================= + Hits 462 482 +20 - Misses 13 19 +6
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please change the commit message so that the first line describes what was done
Thanks!
Code looks good, I'd just ask you to change the commit message so that it includes a one-liner of what it does. Right now it shows up as just a bunch of asterisks in the log.
Will do - sorry about that.
OrderBook now inherits from. This allows easy creation of new types of listeners for the GDAX websocket API. ***************************** *** WebSocketFeedListener *** ***************************** Factored the web socket code away from the actual orderbook implementation. The new WebSocketFeedListener is an abstract base class that handles the websocket itself including: - Connect - Disconnect - Send / Receive messages - Subscribing to channels - Leveraging the trade log file - Async Context Manager calls WebSocketFeedListener inherits from ABC and has a single @AbstractMethod handle_message(self), which must be implemented by subclasses. ****************************** ********* OrderBook ********** ****************************** Removed all WebSocket manipulation from OrderBook and left just the OrderBook implementation. OrderBook now inherits from WebSocketFeedListener and implements handle_message.
96ba72b to
b4d4422
Compare
I amended the commit message. Will that work?
For my own work I needed to modify your code to subscribe to the ticker channel instead of the full orderbook. Your OrderBook code was already well segmented into code that dealt with the Websocket and code that implemented the actual order book. So rather than modify your code I factored the Websocket code out into an abstract base class and then changed your OrderBook class to inherit from it. Now anyone with your API wrapper can easily inherit from WebSocketFeedListener to create their own order book, ticker, or level2 implementations without having to modify your API.
Since the OrderBook still has the same class signature as it originally did, your tests cases all pass without any changes to them. Happy to make changes if you see something you don't like. Details on each change below:
WebSocketFeedListener
Factored the web socket code away from the actual orderbook
implementation. The new WebSocketFeedListener is an abstract base class
that handles the websocket itself including:
WebSocketFeedListener inherits from ABC and has a single @AbstractMethod
handle_message(self), which must be implemented by subclasses.
OrderBook
Removed all WebSocket manipulation from OrderBook and left just the
OrderBook implementation. OrderBook now inherits from
WebSocketFeedListener and implements handle_message.