Skip to content
This repository has been archived by the owner on Mar 9, 2022. It is now read-only.

Websocket.fetch returns reference to underlying data #8

Closed
daikts opened this issue Oct 12, 2020 · 5 comments
Closed

Websocket.fetch returns reference to underlying data #8

daikts opened this issue Oct 12, 2020 · 5 comments

Comments

@daikts
Copy link

daikts commented Oct 12, 2020

Hey there, thanks a lot for building a solid Bybit connector!

I found it very easy to get set up and running, but it seems there is a key usage issue that was counter-intuitive to detect that might be useful to address:

After subscribing to data that gets updated via deltas (such as the orderbook), the returned object is a reference to the underlying Websocket.data structure, which means that if you have code akin to this:

ws = WebSocket(
    endpoint='wss://stream.bybit.com/realtime', 
    subscriptions=['orderBookL2_25.BTCUSD']
)
snapshots = []
for i in range(10):
    snapshot = ws.fetch('orderBookL2_25.BTCUSD')
    snapshots.append(snapshot)
    time.sleep(1)

Then snapshots[0] will actually resolve to the same values as snapshots[1] even if the data has updated between the snapshots, since they resolve to the same object in pybit unless there has been a delete or insert.

I suspect this might be desired behavior if one is only ever interested in the most recent data, but that is not always the case. There would of course be some performance penalty to returning copies each time. Nonetheless, it might be worth highlighting this behaviour in the README if leaving it unchanged.

@verata-veritatis
Copy link
Owner

Hey @daikts, you are absolutely right—I will make this change in the next release. It was originally designed this way for my own needs, but it is definitely not the most desired method of maintaining the data.

@verata-veritatis
Copy link
Owner

verata-veritatis commented Nov 15, 2020

@daikts I've been spending a lot of time doing a bunch of updates/upgrades back and forth and I'm still trying to tackle what you're saying, or rather, trying to understand (sorry it took so long btw...life happened). I think I initially understood what you said differently and now going back to it I think I'm misunderstanding.

You should be receiving a full 25-level orderbook as a list of dictionaries each time, with the dictionaries getting updated each corresponding incoming message. I ran your snippet, printing the snapshot each time, and was able to see different data for each snapshot IF the data was updated with an update, delete, or insert (otherwise the data was the same since no change happened within that 1 second span). I also formed your snippet into a function, returning snapshots so I could probe the list to see if the data was different for each index, which it was. Is this not what you're expecting?

If it isn't behaving like intended, then yes, I can return a .copy() of the keyed value from self.data rather than the value from self.data itself.

@verata-veritatis
Copy link
Owner

WebSocket class will now return a copy of self.data.
aa3311c

@daikts
Copy link
Author

daikts commented Nov 29, 2020

Cool, thanks a lot for the update - returning a copy of self.data was indeed my suggestion, I believe you understood me correctly in the end.

However I see that you're using dict.copy as opposed to copy.deepcopy - I believe we'll still face similar behaviour without the latter, since orderbook data contains lists, which are passed as references when copying shallowly:

In [5]: d = dict(ask_prices=[100,101,102])                                                                                                                                                  

In [6]: result = d.copy()                                                                                                                                                                   

In [7]: d["ask_prices"].pop()                                                                                                                                                               
Out[7]: 102

In [8]: d                                                                                                                                                                                   
Out[8]: {'ask_prices': [100, 101]}

In [9]: result                                                                                                                                                                              
Out[9]: {'ask_prices': [100, 101]}

I haven't scrutinized the source, so there might be other mechanisms to prevent that from happening but might be worth checking!

@verata-veritatis
Copy link
Owner

verata-veritatis commented Jan 16, 2021

@daikts Have you still been having issues with this recently? Figured I'd check in since copy() is working for a few others.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants