feat: add SubscribeTickers func to providers #592
Conversation
RafilxTenfen
requested review from
alexanderbez,
toteki,
adamewozniak and
facundomedica
as code owners
|
@RafilxTenfen does #597 need to be merged first before this? |
| // SubscribeTickers subscribe to all currency pairs and | ||
| // add the new ones into the provider subscribed pairs. |
There was a problem hiding this comment.
nit: try to be consistent and wrap godocs at 80 chars :)
@alexanderbez I don't think it is necessary |
…umee into rafilx/providers-subscribe
There was a problem hiding this comment.
Nice! @adamewozniak & @toteki you wanna give this a view before we merge?
| for _, cp := range cps { | ||
| pairs[iterator] = currencyPairToBinanceTickerPair(cp) | ||
| iterator++ | ||
| pairs[iterator] = currencyPairToBinanceCandlePair(cp) | ||
| iterator++ | ||
| } |
There was a problem hiding this comment.
is cps guaranteed to be an alternating list of Ticker, Candle, Ticker, Candle, etc?
There was a problem hiding this comment.
for the moment, yes
- maybe after tvwap implemented we could only use candles
There was a problem hiding this comment.
oooo I didn't realize this. I'm not really a fan of that approach. It's too brittle IMO. Could we remove that assumption? Maybe just pass two args in instead?
There was a problem hiding this comment.
After tvwap gets merged can we put these back as separate functions?
Candle & Ticker pairs should generally be considered separate, especially since candles can not always be used immediately (you can't calculate tvwap if the only candle you have is one that closed at that exact moment) - sorry I didn't realize this before. Would make this less brittle too.
There was a problem hiding this comment.
Well, these pairs are used only to subscribe to those channels. We could have 2 lists of subscribed pairs and separate them one for each type (Ticker, Candle).
The logic to subscribe in both of those channels was the same. So, in my idea made sense to only iterate through cps ...types.CurrencyPair once
There was a problem hiding this comment.
Would rather iterate twice and keep the code clean and legible 🙏
These aren't big lists anyway
* feat: add SubscribeTickers to binance provider * feat: add SubscribeTickers to huobi provider * feat: add SubscribeTickers to huobi provider * feat: add SubscribeTickers to Okx provider * add subscribe tickers func to changelog * removed unused method * fix: set right amount of topics on reconnect * chore: update godocs * chore: update godocs * chore: update godocs * chore: update godocs Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com> (cherry picked from commit 380ec49)
Description
ref: #181
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!in the type prefix if API or client breaking change