New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OkHttp-based implementation of client AbstractWebSocketTransport #807

Open
nathklei opened this Issue Dec 11, 2018 · 3 comments

Comments

Projects
None yet
2 participants
@nathklei
Copy link
Contributor

nathklei commented Dec 11, 2018

At my organization, we use OkHttp for our http clients. For consistency I wrote an implementation of AbstractWebsocketTransport using the OkHttp WebSocket and WebSocketListener APIs. If this seems generally useful I can work on making the code public.

@sbordet sbordet changed the title OkHttp-based implementation of client AbstractWebssocketTransport OkHttp-based implementation of client AbstractWebSocketTransport Dec 11, 2018

@sbordet

This comment has been minimized.

Copy link
Member

sbordet commented Dec 11, 2018

@nathklei yes it would be useful! - OkHttp has better Android support.
We have tests for various transports, so we can plug in this new one eventually.
FYI, CometD contribution guidelines: https://docs.cometd.org/current/reference/#_contribute_code
OkHttp is also AL2, so licensing wise there is no problem.

nathklei added a commit to nathklei/cometd that referenced this issue Dec 12, 2018

Issue cometd#807 - OkHttp-based implementation of client AbstractWebS…
…ocketTransport

Introduces an OkHttp-based transport. This is useful for environments that
for whatever reason do not have a javax.websocket client.

It comes with two caveats:
1. Message sending can not be made blocking with the current OkHttp APIs. Failures sending will asynchronously result in failure callbacks. A test has been changed to account for this.
2. There is no way to specify a max message size for the OkHttp buffer or for individual messages. A test has been skipped for OkHttp to account for this.

Signed-off-by: Nate Klein <klein.nathaniel@gmail.com>
@nathklei

This comment has been minimized.

Copy link
Contributor

nathklei commented Dec 12, 2018

I've pushed the client here: #808
Feel free to comment on the PR!

@sbordet

This comment has been minimized.

Copy link
Member

sbordet commented Dec 13, 2018

@nathklei Thanks for the PR!
I'm currently busy on #795, so this has to wait a bit, but it's next on the list.

nathklei added a commit to nathklei/cometd that referenced this issue Jan 11, 2019

Issue cometd#807 - OkHttp-based implementation of client AbstractWebS…
…ocketTransport

Introduces an OkHttp-based transport. This is useful for environments that
for whatever reason do not have a javax.websocket client.

It comes with two caveats:
1. Message sending can not be made blocking with the current OkHttp APIs. Failures sending will asynchronously result in failure callbacks. A test has been changed to account for this.
2. There is no way to specify a max message size for the OkHttp buffer or for individual messages. A test has been skipped for OkHttp to account for this.

Signed-off-by: Nate Klein <klein.nathaniel@gmail.com>

nathklei added a commit to nathklei/cometd that referenced this issue Jan 11, 2019

Issue cometd#807 - OkHttp-based implementation of client AbstractWebS…
…ocketTransport

Introduces an OkHttp-based transport. This is useful for environments that
for whatever reason do not have a javax.websocket client.

It comes with two caveats:
1. Message sending can not be made blocking with the current OkHttp APIs. Failures sending will asynchronously result in failure callbacks. A test has been changed to account for this.
2. There is no way to specify a max message size for the OkHttp buffer or for individual messages. A test has been skipped for OkHttp to account for this.

Signed-off-by: Nate Klein <klein.nathaniel@gmail.com>

sbordet added a commit that referenced this issue Jan 23, 2019

Issue #807 - OkHttp-based implementation of client AbstractWebSocketT…
…ransport

Introduces an OkHttp-based transport. This is useful for environments that
for whatever reason do not have a javax.websocket client.

It comes with two caveats:
1. Message sending can not be made blocking with the current OkHttp APIs. Failures sending will asynchronously result in failure callbacks. A test has been changed to account for this.
2. There is no way to specify a max message size for the OkHttp buffer or for individual messages. A test has been skipped for OkHttp to account for this.

Signed-off-by: Nate Klein <klein.nathaniel@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment