Review Board 1.5.4

Allow different Comet implementations by adding an abstraction layer (added Jetty 7 support)

Updated 2 years ago

Marius Danciu Reviewers
wip-marius-217
217 dchenbecker, dpp, marius, charles, joni, atsuhiko, jorge, naftoli, kris, indrajit, alexb
None LiftWeb-archive 
Allow the flexibility of using various implementations of suspend/resume. Currently we have an implementation for Jetty6. If this gets approved the next thing would be to add Jetty 7 support. This would lead to some minor changes regarding the "bail-out" exception as Jetty 7 continuations do not use exceptions any more on suspend. Once we have Jetty 7 support we can start working on Chronium + Jetty 7.0.1 for an early and experimental web sockets integration.

Br's,
Marius
Yes
Ship it!
Ross Mellgren
Posted 2 years ago (January 6th, 2010, 10:16 a.m.)
In general I think the change is good, though my familiarity with the deeper internals of Comet/AJAX support in Lift is light.

Two minor comments:
Shouldn't this be 2010?
  1. Most definitely ! I'll change this.
  2. Marius, no need if you haven't already :) I'll do mass 'search-replace' to 2010 for all the headers.
doc comment says it returns true, but the return type is Option[Any]?
why is the return type Option[Any], anyhow?
  1. It conforms to current types and this represents the state that we're setting on the Continuation object so that we'll have the proper information when the request is resumed. I'll update the doc.
  2. Okay, in that case I think it would be better to rename the method to not have "has" (or "_?"), but it looks like the current API has that name baked in, oh well.
  3. IMO _? is well motivated by the existent of Option. But it's not nailed down yet so we could find better names.
  4. But we can split is since hasSuspendResumeSupport_? has a dual role; to check if suspend resuem is supported and return the objected associated with the Continuation.

def hasSuspendResumeSupport : Boolean

def resumeInfo: Option[Any]

Any objections?
  5. +1
  6. Is it not lift convention to append _? to boolean methods? Lets stick to convention - we use that everywhere else in lift it would be crazy to start doing something different here. 
  7. Anything else happening with this at the moment? 

Cheers, Tim
  8. Yes Tim I'm on it but I haven't had much time lately so the progress was slowed down. I'll try to continue working on it tomorrow and the next week.

Br's,
Marius
Review request changed
Updated 2 years ago (January 24th, 2010, 2:03 a.m.)
added support for jetty7
Review request changed
Updated 2 years ago (January 24th, 2010, 2:40 a.m.)
  • changed from to wip-marius-217

setting up the branch name
Review request changed
Updated 2 years ago (January 24th, 2010, 6:41 a.m.)
  • changed from Allow different Comet implementations by adding an abstraction layer to Allow different Comet implementations by adding an abstraction layer (added Jetty 7 support)

Ship it!
David Pollak
Posted 2 years ago (January 25th, 2010, 11:44 a.m.)
Nifty