sente icon indicating copy to clipboard operation
sente copied to clipboard

Add Jetty 11 adapter

Open alexandergunnarson opened this issue 1 year ago • 10 comments

alexandergunnarson avatar May 09 '24 16:05 alexandergunnarson

Thanks so much for this Alex! I've got my hands a bit full atm with some other open source tasks, but will definitely get this reviewed+merged the next time I'm on batched Sente work.

Will be really nice to have this in 👍

ptaoussanis avatar May 09 '24 20:05 ptaoussanis

No worries! Thanks :)

alexandergunnarson avatar May 10 '24 23:05 alexandergunnarson

@ptaoussanis any idea when this will be merged? Thanks!

awb99 avatar Oct 08 '24 02:10 awb99

@awb99 Are you blocking on this? Would you need a full release, or just a merge?

ptaoussanis avatar Oct 08 '24 05:10 ptaoussanis

I am stuck to a very old sente version that still has jetty support. I would like to update to a more uptodate version of sente that again supports jetty. I have two reasons: 1) old jetty needs websocket handlers to be marked in the config as such (which means my routing table needs to be done effectively twice, or at least my config becomes more convoluted). 2) old sente versions sometimes to not receive disconnected client notification, so I believe I am sometimes sending push event's to clients that are already gone. I could just use a git commit ID to refer to sente via deps.edn if this is what you refer to. I really would like to upgrade ... but I prefer to do it once it is clear the new code works. Thanks!

awb99 avatar Oct 08 '24 06:10 awb99

Thanks, and just to clarify - would you need a full release, or just a review and merge of this PR?

ptaoussanis avatar Oct 08 '24 07:10 ptaoussanis

If you just review and merge, then I can use it in deps.edn projects fine. And if it is helpful to you I could try it in the next 2-4 weeks on some of my apps and report here in case there are issues before you do a full release.

awb99 avatar Oct 08 '24 08:10 awb99

@awb99

And if it is helpful to you I could try it in the next 2-4 weeks on some of my apps and report here in case there are issues before you do a full release.

That'd be great, thank you! 🙏 I've just pushed v1.20.0-SNAPSHOT without fully reviewing or testing this PR except for adding missing dependencies, etc.

Please let me know if/when you've had an opportunity to test.

The reference example includes tools for testing both WebSocket and AJAX features.

ptaoussanis avatar Oct 08 '24 08:10 ptaoussanis

Unfortunately I cannot add snapshot dependencies; most of my libraries are on clojars, and I cannot depend on snapshot dependencies. I was looking for a commit somewhere, a branch or something that I could use? But I cound not find any commits here. What do I overlook? Thanks

awb99 avatar Oct 12 '24 00:10 awb99

I've just added the relevant (temporary, experimental) commits to master 👍

ptaoussanis avatar Oct 12 '24 06:10 ptaoussanis