[fix][ws] Remove unnecessary ping/pong implementation #20733
Merged
Conversation
This file contains 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
michaeljmarshall
added
type/cleanup
michaeljmarshall
requested review from
lhotari,
Technoboy-,
eolivelli and
codelipenghui
michaeljmarshall
changed the title
|
/pulsarbot rerun-failure-checks |
|
Good work @michaeljmarshall . |
mattisonchao
approved these changes
Jul 6, 2023
nodece
approved these changes
Jul 6, 2023
michaeljmarshall
added a commit
that referenced
this pull request
Jul 6, 2023
(cherry picked from commit 33044f0)
michaeljmarshall
added a commit
that referenced
this pull request
Jul 6, 2023
michaeljmarshall
added a commit
that referenced
this pull request
Jul 6, 2023
(cherry picked from commit 33044f0)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
This PR removes an incorrect implementation of the websocket proxy ping/pong feature.
The feature was requested here #10014. When it was implemented in #10035, it was added as an endpoint instead of as support for every endpoint, like our
/producer,/consumer, and/readerendpoints. Notably, these endpoint already have support through the framework, so there is no reason for the/pingpongendpoint. I propose we remove it.Note: since this endpoint should not be used by anyone, I propose we cherry pick this to all active branches.
Modifications
/ws/pingpongand the/ws/v2/pingpongendpoints and all associated codeVerifying this change
A test is modified to prove that the framework gives us the ping/pong support out of the box.
Does this pull request potentially affect one of the following parts:
Removes an unnecessary endpoint.
Documentation
doc-not-neededPing/pong support is not documented here https://pulsar.apache.org/docs/3.0.x/client-libraries-websocket/, so I think we are good to skip docs.
Matching PR in forked repository
PR in forked repository: michaeljmarshall#52