Skip to content
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

feat: add multiplexing config #97

Merged

Conversation

woluohenga
Copy link
Contributor

  1. add ProtocolConfig, put codec in it
  2. add open_multiplexing config

Signed-off-by: woluohenga <[email protected]>
@woluohenga woluohenga force-pushed the feature/modify_api_protocol_config branch from 2ed4aea to 2ecb7b9 Compare February 23, 2023 01:38
@zhaohuabing
Copy link
Member

This PR is the API for #93

// The codec which encodes and decodes the application protocol.
Codec codec = 1;
// is open multiplexing
bool open_multiplexing = 2;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think just multiplexing is clear enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@@ -151,3 +154,10 @@ message Tracing {
envoy.config.trace.v3.Tracing.Http provider = 9;
}

message ProtocolConfig {
Copy link
Member

@zhaohuabing zhaohuabing Feb 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking that we name it ApplicationProtocol and put all configuration about the application protocol in it.

@zhaohuabing
Copy link
Member

Please also add comment to application_protocol and codec to decalare these two fields have been deprecated.

Signed-off-by: woluohenga <[email protected]>
@@ -33,6 +33,7 @@ message MetaProtocolProxy {
string stat_prefix = 1 [(validate.rules).string = {min_len: 1}];

// The name of the application protocol built on top of meta protocol.
// this field has been deprecated, now use protocol
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been deprecated in favor of name in application_protocol.

@@ -46,6 +47,7 @@ message MetaProtocolProxy {
}

// The codec which encodes and decodes the application protocol.
// this field has been deprecated, now use protocol
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been deprecated in favor of codec in application_protocol.

Signed-off-by: woluohenga <[email protected]>
Copy link
Member

@zhaohuabing zhaohuabing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zhaohuabing zhaohuabing merged commit 8cd2ac4 into aeraki-mesh:master Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants