listener: move active_tcp_listener out of conn handler #15553
Conversation
Signed-off-by: Yuchen Dai <[email protected]>
|
@ggreenway Could you take a look? As you suggested this is a code move with a minor change to support the move |
| @@ -52,6 +38,7 @@ void ConnectionHandlerImpl::addListener(absl::optional<uint64_t> overridden_list | |||
| } | |||
| NOT_REACHED_GCOVR_EXCL_LINE; | |||
| } | |||
| // TODO(lambdai): Remove the dependency of ActiveTcpListener. | |||
There was a problem hiding this comment.
@ggreenway This is the why active_tcp_listener.h is required at this PR.
The next PR will erase the dependency.
There was a problem hiding this comment.
That's fine, but the way to pull that in is via deps, not hdrs. I don't see a good reason to do it via hdrs, even temporarily.
There was a problem hiding this comment.
There was a circle dependency if I don't use hdrs :(
Active_tcp_listener target depends on connection_handler_impl target: require ConnectionImplHandler:ActiveListenerImplBase in connection_handler_impl.h.
I can add the active_tcp_listener_header target to avoid this hdrs issue, like below. Hold on
active_tcp_listener_header: connection_handler_impl.h, active_tcp_listener.h
connection_handler_impl : connection_handler_impl.cc, active_tcp_listener_header
active_tcp_listener : active_tcp_listener.cc, active_tcp_listener_header
In active_tcp_listener.h
class ActiveTcpListener : public Network::TcpListenerCallbacks,
public ConnectionHandlerImpl::ActiveListenerImplBase,
There was a problem hiding this comment.
Oh, I see, I didn't understand that there was a circular dependency. Can you add a comment in the BUILD file explaining that, along with a TODO to fix?
There was a problem hiding this comment.
Add the explanation and TODO in BUILD
Signed-off-by: Yuchen Dai <[email protected]>
Signed-off-by: Yuchen Dai <[email protected]>
Signed-off-by: Yuchen Dai <[email protected]>
Signed-off-by: Yuchen Dai <[email protected]>
Signed-off-by: Yuchen Dai <[email protected]>
|
/retest |
|
Retrying Azure Pipelines: |
|
gental ping @ggreenway This PR is now passing all check |
|
Thank you! |
Follow up of #15553. Fixes #15126 Signed-off-by: Yuchen Dai <[email protected]>
Commit Message:
Move active_tcp_listener out of conn handler.
Similar to #15349 but for tcp listener.
Signed-off-by: Yuchen Dai [email protected]
Additional Description:
Risk Level: Low. Mostly just move.
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
Fixes #15126
[Optional Deprecated:]
[Optional API Considerations:]