Introduce new typed HTTPClientUpgrader and WebSocketClientUpgrader#2526
Merged
Lukasa merged 2 commits intoapple:mainfrom Oct 6, 2023
Merged
Introduce new typed HTTPClientUpgrader and WebSocketClientUpgrader#2526Lukasa merged 2 commits intoapple:mainfrom
HTTPClientUpgrader and WebSocketClientUpgrader#2526Lukasa merged 2 commits intoapple:mainfrom
Conversation
d5a4d05 to
d27c1f5
Compare
glbrntt
reviewed
Oct 3, 2023
Contributor
glbrntt
left a comment
There was a problem hiding this comment.
Looks good, mostly nits.
d27c1f5 to
33f8580
Compare
33f8580 to
2118607
Compare
glbrntt
approved these changes
Oct 5, 2023
Contributor
glbrntt
left a comment
There was a problem hiding this comment.
One nit, looks good otherwise
2118607 to
5818fc7
Compare
Member
Author
|
@swift-server-bot test this please |
5818fc7 to
9c8d9ed
Compare
# Motivation In my previous PR apple#2517, I added a new typed `HTTPServerUpgrader` and corresponding implementation for the `WebSocketServerUpgrader`. The goal of those is to carry type information across HTTP upgrades which allows us to build fully typed pipelines. # Modification This PR adds a few things: 1. A new `NIOTypedHttpClientUpgradeHandler` + `NIOTypedHttpClientProtocolUpgrader`. I also moved the state handling to a separate state machine. Similar to the server PR I did not unify the state machine between the newly typed and untyped upgrade handlers since they differ in logic. 2. A new `NIOTypedWebSocketClientUpgrader` 3. An overhauled WebSocket client example. # Result This is the last missing piece of dynamic pipeline changing where we did not carry around the type information. After this PR lands, we can finalize the `AsyncChannel` and async typed NIO pieces.
9c8d9ed to
7b0a2c1
Compare
7b0a2c1 to
014031a
Compare
Lukasa
approved these changes
Oct 6, 2023
Member
Author
|
@Lukasa You mind merging over the expected breaking change CI failure? |
samalone
added a commit
to samalone/websocket-actor-system
that referenced
this pull request
Nov 9, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
In my previous PR #2517, I added a new typed
HTTPServerUpgraderand corresponding implementation for theWebSocketServerUpgrader. The goal of those is to carry type information across HTTP upgrades which allows us to build fully typed pipelines.Modification
This PR adds a few things:
NIOTypedHttpClientUpgradeHandler+NIOTypedHttpClientProtocolUpgrader. I also moved the state handling to a separate state machine. Similar to the server PR I did not unify the state machine between the newly typed and untyped upgrade handlers since they differ in logic.NIOTypedWebSocketClientUpgraderResult
This is the last missing piece of dynamic pipeline changing where we did not carry around the type information. After this PR lands, we can finalize the
AsyncChanneland async typed NIO pieces.