jakartaee / servlet

Jakarta Servlet
https://eclipse.org/ee4j/servlet
Other
253 stars 81 forks source link

Drop support for server push #538

Closed markt-asf closed 4 months ago

markt-asf commented 11 months ago

Given that Chrome has dropped support for server push I'd like to propose the following:

Remove the deprecated methods in Servlet 7.0

arjantijms commented 11 months ago

Some discussion around the removal of push in Chrome: https://reddit.com/r/programming/comments/wrsu94/removing_http2_server_push_from_chrome/?sort=top

gregw commented 11 months ago

I think it is ok to delegate the APIs now, but not yet day that newPushBuilder always returns null.

I.e. let it become an optional feature for a while.

Cheers

On Mon, Aug 14, 2023, 06:53 Arjan Tijms @.***> wrote:

Some discussion around the removal of push in Chrome: https://reddit.com/r/programming/comments/wrsu94/removing_http2_server_push_from_chrome/?sort=top

— Reply to this email directly, view it on GitHub https://github.com/jakartaee/servlet/issues/538#issuecomment-1676459946, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAARJLIWB6CILYHBWMIM2T3XVE5E7ANCNFSM6AAAAAA3KIR5SE . You are receiving this because you are subscribed to this thread.Message ID: @.***>

stuartwdouglas commented 11 months ago

I wonder if people have been using this for non-browser based clients, even though this was fairly useless for browsers you could use it as the basis for a bi-directional RPC framework.

I don't know of anyone actually doing this, but maybe we should be a bit more cautious about deprecating this.

markt-asf commented 11 months ago

@gregw I'd be fine with deprecated and optional (i.e. may always return null) but note that that will still require an update to the TCK (which I'm fine with making).

@stuartwdouglas I'll ask at the IETF what usages folks are aware of but my impression from a quickish scan of the archives is that server push achieved very low levels of adoption. Would you be OK with deprecation and may always return null? In the unlikely event there turns out to be some critical usage of server push and we want to keep it, we can always remove the deprecation.

gregw commented 11 months ago

All,

Of course the other option (horrible and I'm only saying it because it should be mentioned) is that we turn PushBuilders into some kind of 103 Early Hint front end. The "Pushed" request could be served kind of like a HEAD request, i.e. only the headers would be served back in a 103 response.

If there was any significant usage of push builder, then perhaps something like that could be worthwhile.... but I doubt it.

Meanwhile, there should be a standard way of being able to send 103 early hints. Currently Jetty allows a response.sendError(103) to send an early hint with the current headers.

cheers

On Mon, 14 Aug 2023 at 18:52, Mark Thomas @.***> wrote:

@gregw https://github.com/gregw I'd be fine with deprecated and optional (i.e. may always return null) but note that that will still require an update to the TCK (which I'm fine with making).

@stuartwdouglas https://github.com/stuartwdouglas I'll ask at the IETF what usages folks are aware of but my impression from a quickish scan of the archives is that server push achieved very low levels of adoption. Would you be OK with deprecation and may always return null? In the unlikely event there turns out to be some critical usage of server push and we want to keep it, we can always remove the deprecation.

— Reply to this email directly, view it on GitHub https://github.com/jakartaee/servlet/issues/538#issuecomment-1676938256, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAARJLOPNWNMITNLEK2KQM3XVHROJANCNFSM6AAAAAA3KIR5SE . You are receiving this because you were mentioned.Message ID: @.***>

-- Greg Wilkins @.***> CTO http://webtide.com

markt-asf commented 11 months ago

+1 to including support for early hints in the Servlet API. An explicit sendEarlyHints() method?

gregw commented 11 months ago

It would be good to have a way to support 1xx in general.

Jetty also supports 102 processing via sendError. Also pushing -1 is used to abort an in progress response.

All could do with standard API

On Tue, Aug 15, 2023, 19:39 Mark Thomas @.***> wrote:

+1 to including support for early hints in the Servlet API. An explicit sendEarlyHints() method?

— Reply to this email directly, view it on GitHub https://github.com/jakartaee/servlet/issues/538#issuecomment-1678666605, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAARJLK22KHYZNOVFQNPGLDXVM7UZANCNFSM6AAAAAA3KIR5SE . You are receiving this because you were mentioned.Message ID: @.***>

stuartwdouglas commented 11 months ago

I am fine with deprecation. If lots of people complain we can revisit but I think it will be fine.

markt-asf commented 11 months ago

How about this for a plan then:

gregw commented 11 months ago

+1

On Wed, Aug 16, 2023, 17:34 Mark Thomas @.***> wrote:

How about this for a plan then:

  • create a PR to deprecate server push with the Javadoc updated to indicate that newPushBuilder() may return null if the server has opted not to support push
  • create a new issue to discuss generic support for 1xx responses and early hints in particular

— Reply to this email directly, view it on GitHub https://github.com/jakartaee/servlet/issues/538#issuecomment-1680109281, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAARJLJLH3EPAYRKT3URM33XVRZYNANCNFSM6AAAAAA3KIR5SE . You are receiving this because you were mentioned.Message ID: @.***>

pmd1nh commented 4 months ago

Hi,

I posted this in servlet-dev but bring it here to for attention

Hi,

For the Servlet 6.1 HTTP/2 push support,

[Fix #538 - Make HTTP/2 push support optional · jakartaee/servlet@8363ed3 · GitHub](https://github.com/jakartaee/servlet/commit/8363ed338e6875f4e095b6cef8d9571f2fade82d)

Can the API doc remove the last sentence?

"Note that some implementations may opt not to support server push and will therefore always return {@code null} "

That sentence seems to suggest making an optional feature not deprecating.

The Jakarta Spec has tried to stop component specs from saying that a Deprecated API may or may not be implemented.

Regards,

Thanks,

@markt-asf

pmd1nh commented 4 months ago

Reopen this issue

https://jakarta.ee/committees/specification/versioning/

Component Feature

Changes to Component features follow this versioning strategy:

    Major version change - removal of an optional or mandatory feature, or the transition of an existing mandatory feature to being an optional one.
markt-asf commented 4 months ago

The feature has been deprecated and will be removed in a future release.

The possibility of returning null has always existed if "the current connection does not support server push". The final sentence is just making it explicit that one of the reasons the connection might not support it, it that it is not implemented at all.