binance-connector-java icon indicating copy to clipboard operation
binance-connector-java copied to clipboard

refactor(websockets)!: remove WebSocketCallback abstraction to avoid …

Open Hazard4U opened this issue 2 years ago • 3 comments

…losing data between websockets and callbacks

Hazard4U avatar Sep 11 '22 11:09 Hazard4U

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Hazard4U

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

bnbot avatar Sep 11 '22 11:09 bnbot

This PR makes breaking changes, but I think it's important to avoid losing any data between the original websocket and the callbacks given to the user.

My use case was as follows: When the socket failes due to java.net.SocketException: Connection reset, I was not able to tell if it failed due to the connection reset or some other failure. Now you can catch the throwable in onFailure and find out if a restart is needed.

Hazard4U avatar Sep 11 '22 11:09 Hazard4U

Hi , when are u planning to merge this PR ? need this feature

naserkaka avatar Sep 26 '22 21:09 naserkaka

@Hazard4U: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

bnbot avatar Oct 19 '22 04:10 bnbot

@Hazard4U: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

bnbot avatar Oct 20 '22 04:10 bnbot

@Hazard4U: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

bnbot avatar Oct 21 '22 04:10 bnbot

@Hazard4U: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

bnbot avatar Oct 22 '22 04:10 bnbot

@Hazard4U: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

bnbot avatar Oct 23 '22 04:10 bnbot

@Hazard4U: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

bnbot avatar Oct 24 '22 04:10 bnbot

@Hazard4U: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

bnbot avatar Oct 25 '22 04:10 bnbot

@Hazard4U: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

bnbot avatar Oct 26 '22 04:10 bnbot

@Hazard4U: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

bnbot avatar Oct 27 '22 04:10 bnbot

@Hazard4U: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

bnbot avatar Oct 28 '22 04:10 bnbot

Thank you @Hazard4U! We were finally able to get into this and now planning to publish it to v3.0.0rc2. Since this PR is already from a while ago and to solve the conflicts here we would require you to update your repo's branch, we've decided to create https://github.com/binance/binance-connector-java/pull/81 instead with your commits in it to speed things up. :)

This being said, I think it's safe to close this PR now.

aisling-2 avatar May 16 '23 06:05 aisling-2