Skip to content
This repository has been archived by the owner on Jun 8, 2020. It is now read-only.

Fix gdax disconnect NPE/Bitmex execution reports/bug-fix rollup #191

Merged
merged 98 commits into from
Oct 24, 2018
Merged

Fix gdax disconnect NPE/Bitmex execution reports/bug-fix rollup #191

merged 98 commits into from
Oct 24, 2018

Conversation

badgerwithagun
Copy link
Collaborator

@badgerwithagun badgerwithagun commented May 20, 2018 •
edited
Loading

This also rolls in a rebased version of #141 which looked related enough to do at the same time.

Also includes:

  • "L1" support for execution reports on BitMex. This is in-line with the approach for GDAX on Gdax adding user channel #160, just missing the L2 support that Gdax adding user channel #160 adds on top. I'm happy with this - I am implementing Binance on the same basis (L1 support first, then work out how to create L2-generic support which combines GDAX and BitMex under the same API later). However, I don't use my bot code on BitMex so I can't easily test that it works.
  • Bitfinex proxy support
  • Various updated dependencies
  • Performance improvements (mainly ensuring we re-use ObjectMapper instances and use them more efficiently (use treeToValue())
  • Fix for non-3-character tickers on Binance
  • API support and support on Bitmex for testnet/sandbox - I'll be adding support for GDAX as soon as this is merged.
  • Reconnect/connection success events for some exchanges

dozd and others added 17 commits November 23, 2017 16:17
@dozd
Merge tag 'xchange-stream-parent-4.3.0'
1f6369e 
[maven-release-plugin] copy for tag xchange-stream-parent-4.3.0
@dozd
Merge tag 'xchange-stream-parent-4.3.1'
6bf8087 
[maven-release-plugin] copy for tag xchange-stream-parent-4.3.1
Merge tag 'xchange-stream-parent-4.3.2'
153a9da 
[maven-release-plugin] copy for tag xchange-stream-parent-4.3.2
@somerandomam
xhange 4.3.4, binance fix, okcoin test fix, NettyStreamingService tuning
889bd84
@somerandomam
reconnection fix
8b1af64
@somerandomam
shutdown netty executor on disconnect
f750cf8
@somerandomam
[maven-release-plugin] prepare for next development iteration
4abae75
@somerandomam
xhange 4.3.4, binance fix, okcoin test fix, NettyStreamingService tuning
d91199e
@somerandomam
reconnection fix
81884f4
@somerandomam
shutdown netty executor on disconnect
8142e4c
@somerandomam
Merge branch 'develop' of https://github.com/trojanflash/xchange-stream…
c12d08b 
… into develop
@jkolobok
bitrich-info#163 Fixes connection to bitmex and book updates
d34846f
@jkolobok
Merge remote-tracking branch 'remotes/upstream/develop' into develop
e423885
@jkolobok
bitrich-info#163 Fixes npe and channel disconnect if update arrives …
3529bb9 
…before partial
@jkolobok
bitrich-info#163 Fixes npe in example when book is empty
a7aac95
@badgerwithagun
Merge remote-tracking branch 'remotes/trojanflash/develop' into lates…
e7c1412 
…tandgreatest

# Conflicts:
#	pom.xml
#	service-netty/src/main/java/info/bitrich/xchangestream/service/netty/NettyStreamingService.java
#	service-netty/src/main/java/info/bitrich/xchangestream/service/netty/WebSocketClientHandler.java
#	xchange-binance/src/main/java/info/bitrich/xchangestream/binance/dto/TickerBinanceWebsocketTransaction.java
#	xchange-okcoin/src/test/java/info/bitrich/xchangestream/okcoin/OkCoinStreamingMarketDataServiceTest.java
@badgerwithagun
[gdax] NPE protection during disconnection
338212e
@badgerwithagun
Copy link
Collaborator Author

My change is just the null checks. Everything else is from #141.

@badgerwithagun
Copy link
Collaborator Author

OK, the changes from #141 are causing me some problems when using the same market data service repeatedly (disconnect/reconnect) since the executor's been shut down. It's also not waiting for shutdown before signalling completion/ I have a fix coming on this PR but want to test it a bit more.

@badgerwithagun badgerwithagun mentioned this pull request May 21, 2018
Closed
@badgerwithagun
Ensure eventLoopGroup has shutdown before returning disconnect comple…
aec06a2 
…tion.

Ensure that disconnecting when already disconnected returns onCompletion.
Allow reconnection to occur afterwards.
@badgerwithagun badgerwithagun mentioned this pull request May 24, 2018
Open
@badgerwithagun badgerwithagun mentioned this pull request Oct 8, 2018
Closed
@Flemingjp Flemingjp added this to the 4.3.11 milestone Oct 8, 2018
@badgerwithagun
Copy link
Collaborator Author

Broken by the upgrade to 4.3.11. I don't use Bitmex so can't help there.

This was referenced Oct 8, 2018
Closed
Closed
@Flemingjp Flemingjp mentioned this pull request Oct 9, 2018
Open
@badgerwithagun badgerwithagun mentioned this pull request Oct 14, 2018
Merged
@declan94
Copy link
Collaborator

declan94 commented Oct 17, 2018 •
edited
Loading

@Flemingjp After upgrade to XChange 4.3.11, all the examples for Bitmex have broken.
@badgerwithagun I'm using bitmex and I'm willing to fix these problems. But I can't modify this PR directly. Maybe we should first review about Binance, after that, I can make a new PR rebase on this one.

What do you think?

@badgerwithagun
Copy link
Collaborator Author

@declan94: yes, I was thinking doing so might help things along. All the Bitmex stuff here was rolled in as part of other peoples' PRs a long time ago and I wasn't comfortable having a bunch of code here I hadn't tested anyway.

I'm happy to revert the Bitmex changes on this branch (probably tonight, Euro time) leaving us with a clean review, and if you could then create a rebased branch and create a new PR we can go from there.

@badgerwithagun
Copy link
Collaborator Author

Right, sorry about the delay. Been busy with other things. I'm going to try and surgically extract the Bitmex changes now.

@badgerwithagun
Copy link
Collaborator Author

badgerwithagun commented Oct 23, 2018 •
edited
Loading

OK, I've removed all the Bitmex-specific changes. These were removed in 55513ff.

Hopefully this is good to merge now! @declan94 , if you could submit your review, I'll fix up anything you spot.

Once that's done, I'll resubmit the Bitmex changes separately for someone who uses BitMex to finish off. Just a reminder, only about 20% of the changes here are actually my work (and none of the Bitmex stuff).

@Flemingjp
Copy link
Collaborator

@badgerwithagun So this is just GDAX work now?

@badgerwithagun
Copy link
Collaborator Author

badgerwithagun commented Oct 24, 2018 •
edited
Loading

@Flemingjp : updated list:

  • Bitfinex proxy support
  • Various updated dependencies
  • Performance improvements (mainly ensuring we re-use ObjectMapper instances and use them more efficiently (use treeToValue())
  • Fix for non-3-character tickers on Binance
  • Reconnect/connection success events for some exchanges (GDAX being the main one)

@Flemingjp
Copy link
Collaborator

@badgerwithagun Ready to merge this PR if all is good on your end

@badgerwithagun
Copy link
Collaborator Author

@Flemingjp yep.

@Flemingjp Flemingjp merged commit 368454c into bitrich-info:develop Oct 24, 2018
This was referenced Oct 25, 2018
Closed
Closed
badgerwithagun pushed a commit to badgerwithagun/xchange-stream that referenced this pull request Oct 25, 2018
@Foat @badgerwithagun
Restore the Bitmex Execution reports code which was cut out of bitric…
04b3d10 
…h-info#191.

This is has some issues with the latest codeset (in particular, it doesn't compile against XChange 4.3.11).
It needs the attention of someone who actually uses BitMex.
@badgerwithagun badgerwithagun mentioned this pull request Oct 25, 2018
Merged
@badgerwithagun badgerwithagun deleted the fix-gdax-disconnect-npe branch October 28, 2018 16:09
@pchertalev pchertalev mentioned this pull request Nov 14, 2018
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Flemingjp Flemingjp Flemingjp approved these changes

Assignees
No one assigned
Labels
bug-fix
Projects
None yet
Milestone
4.3.11
Development

Successfully merging this pull request may close these issues.
10 participants
@badgerwithagun @Dima-HAN @Flemingjp @declan94 @dozd @somerandomam @jkolobok @Foat @leoxtc @kaptravis