Kraken: Fix wsCancelOrders not erroring with order id #1505
Conversation
thrasher-
added
review me
gbjk
force-pushed
the
feature/kraken_cancel_orders
branch
3 times, most recently
from
1b26681
to
f34ffc6
Compare
|
I'm not liking those failures in kraken's |
gloriousCode
added
pending response
|
TestWsOpenOrders errors were unrelated. This is good to review. |
gbjk
added
review me
There was a problem hiding this comment.
Thanks for making those changes! tACK!
As an extra, would you consider this diff? Otherwise, I can just put on another PR. I just wanted to get rid of the func setupWsTests while I was here
|
Already in the next upcoming branch. I've got significant testing of Subscribe in there without mocking, so I'm on the fence about adding a mock for it if we don't have to ( perversely ). Anyway, Let's touch on this when that PR lands. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1505 +/- ##
==========================================
- Coverage 37.83% 36.02% -1.82%
==========================================
Files 411 411
Lines 147204 176906 +29702
==========================================
+ Hits 55699 63728 +8029
- Misses 83733 105391 +21658
- Partials 7772 7787 +15
|
gbjk
force-pushed
the
feature/kraken_cancel_orders
branch
from
ebf4876
to
1849749
Compare
|
Rebased and ready to roll 😄 |
gbjk
force-pushed
the
feature/kraken_cancel_orders
branch
2 times, most recently
from
ebf4876
to
bc84123
Compare
We were using the "cancel many" facility of the Kraken api. However since that doesn't actually report errors individually, it seems saner to just multiplex over it. We were going to get N+ responses anyway. Might as well send N+ requests
gbjk
force-pushed
the
feature/kraken_cancel_orders
branch
from
bc84123
to
e7e5a5e
Compare
We were using the "cancel many" facility of the Kraken api. However since that doesn't actually report errors individually, it seems saner to just multiplex over it.
We were going to get N+ responses anyway. Might as well send N+ requests
One thing I'm unhappy about is the change of Key/Secret for this to work.
I really don't like that there's special values for the auth values which get silently ignored, such as Test, Mock, Key, Secret, etc.
So I've ended up using something adhoc, and putting a require in the test to ensure it's obvious if the auth-conn isn't connected.
Type of change
How has this been tested