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.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
xds: unify client and server handling HttpConnectionManager #8228
xds: unify client and server handling HttpConnectionManager #8228
Changes from all commits
18060b4
0883219
6753f15
fbc3bd1
c825bd0
308da0b
4837170
57ec58d
bb967b3
f49edd0
361096b
2fa37bb
38a6846
b6411ae
4f779f7
90eab59
d9c674b
b2ee048
d3ac75c
4ef3425
003c535
8cbb28f
9001c89
ed23f12
163d636
8b845a3
f052e72
f735616
4b000a5
e3e885e
bd26751
7136ec5
09ee39e
2ae6b34
9ad1e94
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not from this PR but why use
enum
when clearly this type is not used to enumerate constants?enum
works given how this is used but I wouldn't think of theRouterFilter
as an enum.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Java, enum types are considered to be a special type of class. When creating an enum class, the compiler will create instances (objects) of each enum constants. So enum can be an easy trick to create singletons.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the server side there is no such thing as one httpConnectionManager in the LDS because you can have more than one httpConnectionManager due to the filterChain. What's the plan for properly supporting the server side semantics?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See
EnvoyServerProtoData.FilterChain
: OneFilterChain
has oneHttpConnectionManager
. The server side will only useLdsUpdate.listener
field, the rests are for client side.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now
LdsUpdate
is an oneofHttpConnectionManager
(for client-side) orListener
(for server-side).