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

binance: use built-in anext() add note about new ws ep URL, fix agen streaming within NoBsWs usage #473

Merged
merged 4 commits into from
Apr 12, 2023

Conversation

goodboy
Copy link
Contributor

@goodboy goodboy commented Feb 28, 2023

Not sure why this ain't workin for me (but maybe someone else can test
to verify?) but I tried updating the latest documented WS endpoint and
it doesn't seem to ever work 😂

Further this starts using the new built-in anext() routines instead of
the dunders we had to use prior.


ALSO updates the streamer async-gen with an aclosing() which is
super critical to avoid trio crashes on re-connection logic!

INSTEAD uses @trio_util.trio_async_generator which actually
solves the underlying issue of allowing an agen to work through multiple
cancel scopes as is needed by our NoBsWs reconnection logic.. => in 609b91e

@goodboy goodboy added integration external stack and/or lib augmentations broker-backend `brokerd`/`datad` related backend tech labels Feb 28, 2023
@goodboy
Copy link
Contributor Author

goodboy commented Feb 28, 2023

Ahh shoot, i guess you have to checkout and uncomment the other ep url line 😂

here's their docs on this: https://developers.binance.com/docs/binance-trading-api/websocket_api#general-api-information

no idea what's up, but obviously as per this patch, we were using 'wss://stream.binance.com/ws'

@goodboy goodboy marked this pull request as ready for review March 13, 2023 19:38
@goodboy goodboy changed the title binance: use built-in anext() add note about new ws ep URL binance: use built-in anext() add note about new ws ep URL, fix agen streaming within NoBsWs usage Mar 17, 2023
@goodboy goodboy changed the title binance: use built-in anext() add note about new ws ep URL, fix agen streaming within NoBsWs usage binance: use built-in anext() add note about new ws ep URL, fix agen streaming within NoBsWs usage Mar 17, 2023
Apparently it will likely fix our `trio`-cancel-scopes-corrupted crash
when we try to let our `._web_bs.NoBsWs` do reconnect logic around
the asyn-generator implemented data-feed streaming routines in `binance`
and `kraken`.  See the project docs for deatz; obvs we add the lib as
a dep.
Copy link
Contributor

@jaredgoldman jaredgoldman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@goodboy goodboy merged commit 70db20b into master Apr 12, 2023
@goodboy goodboy deleted the binance_ws_ep_update branch April 12, 2023 23:53
@goodboy
Copy link
Contributor Author

goodboy commented May 26, 2023

Turns out this didn't end up workin that well 😂

Had to do a full rewrite in 59743b7@#489

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
broker-backend `brokerd`/`datad` related backend tech integration external stack and/or lib augmentations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants