Skip to content

Conversation

@dcantah
Copy link
Member

@dcantah dcantah commented Feb 16, 2023

We pass in a callback using the ttrpc.WithOnClose functionality
for shims that use ttrpc, but with the newly added ability to use
GRPC for shims this was left as a follow-up.

This change introduces a new grpcConn wrapper type for the connection
that exposes a method to get notified when the users callback has run,
the same in functionality as TTRPC's UserOnCloseWait. The callback
gets passed in in a new grpcDialContext function that will:

  1. Dial the connection as normal
  2. Spin off a goroutine that will monitor the connections state
    until it transitions to idle or shutdown and will then run the
    callback.

@dcantah dcantah requested a review from mxpv February 16, 2023 00:17
@dcantah dcantah added this to the 1.7 milestone Feb 16, 2023
@dcantah dcantah force-pushed the grpc-shim-callbackfn branch from a3b0e27 to 3cf03a2 Compare February 16, 2023 06:26
@dcantah
Copy link
Member Author

dcantah commented Feb 16, 2023

@mxpv Addressed feedback, thanks!

@dcantah dcantah force-pushed the grpc-shim-callbackfn branch 2 times, most recently from 7464866 to 9224f88 Compare February 17, 2023 01:11
@dcantah dcantah requested review from fuweid and mxpv February 17, 2023 01:43
Copy link
Member

@fuweid fuweid left a comment

Choose a reason for hiding this comment

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

LGTM on green

@fuweid

This comment was marked as outdated.

2 similar comments
@fuweid
Copy link
Member

fuweid commented Feb 17, 2023

/retest

@fuweid
Copy link
Member

fuweid commented Feb 17, 2023

/retest

@mxpv
Copy link
Member

mxpv commented Feb 20, 2023

/retest

@dcantah dcantah force-pushed the grpc-shim-callbackfn branch from 9224f88 to c15b75c Compare February 21, 2023 02:21
We pass in a callback using the ttrpc.WithOnClose functionality
for shims that use ttrpc, but with the newly added ability to use
GRPC for shims this was left as a follow-up. It doesn't seem like
grpc-go has anything similar so some options (that I could see) are:

This change introduces a new grpcConn wrapper type for the connection
that exposes a method to get notified when the users callback has run,
the same in functionality as TTRPC's `UserOnCloseWait`. The callback
gets passed in in a new `grpcDialContext` function that will:

1. Dial the connection as normal
2. Spin off a goroutine that will monitor the connections state
until it transitions to idle or shutdown and will then run the
callback.

Signed-off-by: Danny Canter <[email protected]>
@dcantah dcantah force-pushed the grpc-shim-callbackfn branch from c15b75c to 4278fbb Compare February 21, 2023 02:26
containerd#8143 added an alias for
logrus.Fields and moved over most usages to this alias, but there was
one straggler.

Signed-off-by: Danny Canter <[email protected]>
@dcantah
Copy link
Member Author

dcantah commented Feb 21, 2023

@mxpv There was one last logrus.Fields usage in here it seems. Since I changed my newly added log to your alias you added I just added a commit on top to swap over this last straggler. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants