Subscription Pairs and Encapsulation #1501
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1501 +/- ##
==========================================
- Coverage 36.04% 36.02% -0.03%
==========================================
Files 411 415 +4
Lines 176906 177039 +133
==========================================
Hits 63774 63774
- Misses 105351 105475 +124
- Partials 7781 7790 +9
|
gbjk
force-pushed
the
feature/sub_pairs
branch
5 times, most recently
from
3149dba
to
d7e5c4c
Compare
gbjk
added
reconstructing
gbjk
force-pushed
the
feature/sub_pairs
branch
2 times, most recently
from
d2622f0
to
6774e9d
Compare
gbjk
force-pushed
the
feature/sub_pairs
branch
2 times, most recently
from
73b8699
to
c3250d3
Compare
If the exchange passed in already has a websocket, don't clobber it
instead of errChannelAlreadySubscribed
Given that some subscriptions have multiple pairs, support that as the standard.
We deliberately use Equal over Len to avoid spamming the contents of large Slices
| @@ -212,11 +214,10 @@ func New(input string) (Item, error) { | |||
| return USDCMarginedFutures, nil | |||
| case options, "option": | |||
| return Options, nil | |||
| case all: | |||
There was a problem hiding this comment.
This is my own personal review comment to say that I have reviewed the manner of all and am satisfied. This is to prevent me from reviewing it out of concern every single time I review this PR
| if s.Key == nil { | ||
| s.Key = DefaultKey{ | ||
| Channel: s.Channel, | ||
| Asset: s.Asset, | ||
| Pair: s.Pair, | ||
| } | ||
| s.Key = &ExactKey{s} | ||
| } | ||
| return s.Key |
There was a problem hiding this comment.
This looke like it should have mutex use now
| if k, ok := key.(MatchableKey); ok { | ||
| return k.String() | ||
| } | ||
| return fmt.Sprintf("%v: %s", key, ExactKey{s}.String()) |
There was a problem hiding this comment.
Looks like a potential race zone
There was a problem hiding this comment.
Yeah. Not particularly easy to get around, either.
Fixed 913fb2d
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1501 +/- ##
==========================================
- Coverage 36.04% 36.02% -0.03%
==========================================
Files 411 415 +4
Lines 176906 177039 +133
==========================================
Hits 63774 63774
- Misses 105351 105475 +124
- Partials 7781 7790 +9
|
This work starts the process of disentangling subscriptions from websocket, since a subscription is an exchange concept, and we want the collection of subscriptions separate from each websocket so we can start to separate auth, non-auth, assets, private and multiplexed websockets
For some exchanges we've immediately made full use of this.
For others we've ported over the existing one-pair method, and will revist it when we tackle subscription configuration
Type of change
How has this been tested