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

bybit: add trade spot and trade option websocket msgspec schemas #1570

Merged
merged 4 commits into from
Mar 30, 2024

Conversation

davidsblom
Copy link
Member

@davidsblom davidsblom commented Mar 30, 2024

Pull Request

Add msgspec schema's for trade messages for spot and option instruments, due to the L field being unique to futures and id being unique to options.

Type of change

Delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How has this change been tested?

Locally listening to trade and quote ticks for the Bybit exchange.

@cjdsellers
Copy link
Member

Thanks for the help @davidsblom!

I think even more efficient if we just define some optional fields for the trade message, rather than the entirely separate messages per instrument. I'll merge as is though if this was working for you, and we can always refactor.

@cjdsellers cjdsellers merged commit 434d14f into nautechsystems:develop Mar 30, 2024
11 checks passed
@davidsblom
Copy link
Member Author

Great! Optional fields are needed much cleaner.

The important fix is the use of the partial instead of a lambda. The lambda functions were being overwritten somehow when using multiple product types. Any idea how that is even possible?

@davidsblom
Copy link
Member Author

Found the answer here: https://stackoverflow.com/questions/50298582/why-does-python-asyncio-loop-call-soon-overwrite-data

When the lambda function is called, it will use the values bound by those names in the outer scope at that time of the call, not the values they had when the lambda was defined.

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

Successfully merging this pull request may close these issues.

2 participants