Skip to content
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

WebSockets support #404

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

worryg0d
Copy link

Introduction

Hello team,
I've been working on behalf of shotover-proxy project so we want to enable this Cassandra driver to work with Cassandra via other transports. For instance, apps can connect to Cassandra via WebSockets using shotover-proxy. For more information follow its repo.

Some environments support only HTTP transport like browsers, some lambda executors, etc. So, we want to make Cassandra to be reachable via WebSockets.

WebSocket support

Added WebSocket support to the driver by creating a wrapper on ws.WebSocket that satisfies net.Socket methods used by the driver.

Usage

To use WebSocket user should define options.transport with one of the following values:

  • WebSocket - for no secure connection
  • SecureWebScoket - for secure connection over TLS/SSL

Configuration

To configure WebSocket client use options.webSocketOptions property of the Cassandra client constructor. It accepts the same options as a default WebSocket client (follow doc).

Also, a user should provide subprotocols by setting them to options.webSocketOptions.protocols if using such proxy as shotover.

@worryg0d worryg0d force-pushed the websocket-support branch 4 times, most recently from 9557f16 to e550dc9 Compare July 18, 2023 09:59
@absurdfarce
Copy link
Collaborator

Greetings @worryg0d and thanks for the contribution! Apologies for taking so long to get back to you.

Have you signed the Contributor License Agreement for contributions to DataStax open source projects? If not you can find it at https://cla.datastax.com/. Thanks!

@absurdfarce absurdfarce self-requested a review August 23, 2023 21:09
@worryg0d
Copy link
Author

Greetings @worryg0d and thanks for the contribution! Apologies for taking so long to get back to you.

Have you signed the Contributor License Agreement for contributions to DataStax open source projects? If not you can find it at https://cla.datastax.com/. Thanks!

Hello @absurdfarce. Thanks a lot for your feedback! Yes, I have signed it.

@absurdfarce
Copy link
Collaborator

Hey @worryg0d , thanks for letting me know!

As an update here: we're working to get version 4.7.0 of the nodejs driver out the door with a few features we need for various other work. Given the tight time frame of that work I wasn't able to give this PR the attention it needs so it won't be included in that release. My plan is to dig into this more after the release, although in all honesty our team has a number of high-priority issues waiting for us.

So I guess what I'm saying is that I haven't forgotten you... but it might be a little while yet. :(

All of that said, thanks for the PR! It's an interesting idea and I'm looking forward to seeing what you've implemented here, how it works with shotover and what (if any) implications there are for other use cases.

@worryg0d
Copy link
Author

Hey @worryg0d , thanks for letting me know!

As an update here: we're working to get version 4.7.0 of the nodejs driver out the door with a few features we need for various other work. Given the tight time frame of that work I wasn't able to give this PR the attention it needs so it won't be included in that release. My plan is to dig into this more after the release, although in all honesty our team has a number of high-priority issues waiting for us.

So I guess what I'm saying is that I haven't forgotten you... but it might be a little while yet. :(

All of that said, thanks for the PR! It's an interesting idea and I'm looking forward to seeing what you've implemented here, how it works with shotover and what (if any) implications there are for other use cases.

Hello @absurdfarce, I'm glad to hear you again.

I understand you have to deal with some high-priority things, so I don't mind to wait for. Thanks for letting us know.

We're waiting for your review feedback and we hope everything will be fine.

@benbromhead
Copy link

Just wanted to check in on whether there is some bandwidth for this PR to get reviewed (might need to be updated)?

@worryg0d worryg0d force-pushed the websocket-support branch from e550dc9 to abfb010 Compare March 21, 2024 09:40
@worryg0d worryg0d force-pushed the websocket-support branch from abfb010 to db37df0 Compare March 21, 2024 09:53
@absurdfarce
Copy link
Collaborator

Hey @benbromhead , thanks for checking in! I know this PR has been sitting for a while and I appreciate that there's a desire to continue to work on it. Unfortunately we do have an enormous backlog of work across the drivers team and at the moment it's going in the wrong direction. Please continue to be patient with us and I'll try to get to this PR when I circle back to address a few other Node.js PRs/issues I've got lined up.

@benbromhead
Copy link

Fair enough.

Are there any PRs / outstanding issues you would like some external assistance on? Happy to help out!

@absurdfarce
Copy link
Collaborator

absurdfarce commented Apr 29, 2024

Thanks for offering @benbromhead! There actually are a couple items I could use some help on if you have the time.

I have a planning document for the next release (tentatively 4.7.3, although obviously that could change) here. I'm not super-worried about the vector-related tickets (NODEJS-666 and NODEJS-667); I have a pretty good handle on the node.js support for vectors and am pretty sure I can tackle those myself. But....

I wouldn't mind another set of eyes on NODEJS-626. I'm pretty sure what's going on there is some confusion with the name RangeError (i.e. in some cases it's being resolved to a different entity)... but reproducing and/or nailing it down further has been difficult. If you have any ideas that might help here I'd certainly welcome them.

It would also be useful to have somebody else take a look at the PR to remove the @types/long dependency. It seems fairly straightforward to me but I readily admit that I don't know much about TypeScript... so I'm not super-confident in understanding the full implications of this change. If this is something you know more about I'd welcome any additional comments you might have.

Those are the two big ones that spring to mind; if others come up I can be sure to let you know!

@worryg0d
Copy link
Author

worryg0d commented May 9, 2024

Hi @absurdfarce, I've looked at PR #417 as you asked for.

As far as I know, node_modules/@typesstores type definitions files .d.ts for npm packages which don't provide them itself. However, since version 4.0.0, the long package includes its own type definitions, so it is may likely be deleted.

Regarding the reported error, TS2688 occurs when tsc is unable to locate.d.ts files in node_modules/@types/long and the node_modules/long directory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants