network: rename SocketAddressProvider as ConnectionInfoProvider #17717
Conversation
soulxu
requested review from
alyssawilk,
antoniovicente,
asraa,
ggreenway,
jmarantz,
lizan,
snowp and
zuercher
as code owners
|
/retest |
|
Retrying Azure Pipelines: |
|
/retest |
|
Retrying Azure Pipelines: |
|
I realise the changes in the PR are renames (only?), but I think this is too large to review. Perhaps consider renaming the classes but leave the variable names as they are? Those can be renamed in smaller chunks, or as drive-bys? Should |
yes, it is only about rename. also agree it is huge...terrible for review. I can separate them. Thanks for review this terrible PR!
Yes, it could be, do you want me do it in separate PR? |
|
It is still huge, I'm trying to only rename the classname without the member name first. |
Don't think it would increase the number of changes that much, I think it's probably ok to do it here. It's up to you, I think either way is ok though. |
Perhaps a dedicated PR would be better after all. I realised a bunch BUILD files woould need to be updated. that would make this PR harder to review... |
…InfoProvider Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
soulxu
force-pushed
the
rename_address_provider
branch
from
ce64369 to
17da5f0
Compare
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
|
lgtm, thanks! |
| @@ -194,8 +194,8 @@ class Connection : public Event::DeferredDeletable, | |||
| /** | |||
| * @return the address provider backing this connection. | |||
| */ | |||
| virtual const SocketAddressProvider& addressProvider() const PURE; | |||
| virtual SocketAddressProviderSharedPtr addressProviderSharedPtr() const PURE; | |||
| virtual const ConnectionInfoProvider& addressProvider() const PURE; | |||
There was a problem hiding this comment.
Your plan is to rename the function name and other things in follow ups, right?
/wait-any
There was a problem hiding this comment.
yes, if we do that in one pr, it will be thousand lines. It is hard to review #17717 (comment)
I will have one PR for rename the function name, and another PR for rename the variable name.
* main: (32 commits) Stop processing pending H/2 frames if connection transitioned to the closed state http2: limit use of deferred resets in the http2 codec to server-side connections Abort filter chain iteration on local reply Reject or strip fragment from request URI ext-authz: merge duplicate headers from client request in check request common: introduce stable logger /w examples in DNS (envoyproxy#17772) route: fast return when route matches failed (envoyproxy#17769) owners: add owners for dubbo proxy network filter (envoyproxy#17820) config/router/tcp_proxy/options: v2 API, boosting and --bootstrap-version CLI removal. (envoyproxy#17724) coverage: revert the limit http/cache to 92.6. (envoyproxy#17817) network: rename SocketAddressProvider as ConnectionInfoProvider (envoyproxy#17717) test: bumping coverage (envoyproxy#17757) conn_pool: Minor cleanups to ConnPoolBaseImpl (envoyproxy#17710) Split VaryHeader into VaryAllowList and VaryUtils to organize vary-related logic (envoyproxy#17728) ext_proc: Make tests more resilient to IPv6 support (envoyproxy#17784) Remove invlaid backquote from doc (envoyproxy#17797) rocketmq: move to contrib (envoyproxy#17796) kafka: upstream kafka facade in mesh-filter (envoyproxy#17783) ecds: create shared base class for DynamicFilterConfigProviderImpl (envoyproxy#17735) Change log level from debug to trace (envoyproxy#17774) ... Signed-off-by: Michael Puncel <mpuncel@squareup.com>
- renamed `Envoy::Network::SocketAddressProvider` to `Envoy::Network::ConnectionInfoProvider` as per envoyproxy/envoy#17717. - removed a boolean argument in a call to `Envoy::MessageUtil::loadFromFile` as per envoyproxy/envoy#17724. - removing reference to and presence of the unknown proto field `Envoy::ProtobufWellKnown::OriginalTypeFieldNumber` after Envoy deleted it in envoyproxy/envoy#17724. - updated the command line in `README.md` to reflect Envoy's removal of flags related to bootstrap version. - moving Envoy configurations used in integration tests off the deprecated field `envoy.config.bootstrap.v3.Admin.access_log_path`. - no changes to `.bazelrc`, `.bazelversion`, `run_envoy_docker.sh`. Signed-off-by: Jakub Sobon <mumak@google.com>
Commit Message: network: rename SocketAddressProvider as ConnectionInfoProvider
Additional Description:
Since SocketAddressProvider isn't only include the socket address info, also include the information for connection, like connection id, SSL connection info and SNI name. So rename it as ConnectionInfoProvider.
Risk Level: low
Testing: all
Docs Changes: n/a
Release Notes: n/a
Part of #17168