cisco-ie / cisco-gnmi-python

CLI and library wrapping gNMI functionality to ease usage with Cisco implementations in Python programs.
https://pypi.org/project/cisco-gnmi/
Apache License 2.0
40 stars 18 forks source link

The grpc channel is not closing. #56

Closed miott closed 4 years ago

miott commented 4 years ago

If the TSL/authentication parameters are not valid, the channel remains open retrying the authentication. The channel is not accessible from the subclasses of Client and without having access to the "Channel.close" method or having the ability to delete the channel from memory, the only way to completely shutdown is to exit the Python interpreter.

akshaykhushu commented 4 years ago

Yes, I also think there should be a way of closing the channel through the Client class. It doesn't make sense to restart the Python interpreter to shutdown the channel, since the channel class is not accessible from Client

remingtonc commented 4 years ago

Addressing in branch expose_channel.

remingtonc commented 4 years ago

Initially hoped that the stub might have channel references in the object, but the stub methods appear to compose on the channel (eg). Cool pattern. Storing a reference to the channel in the object does seem like the best methodology as accessing via stub doesn't seem very viable.

remingtonc commented 4 years ago

I see 2 methodologies for this:

  1. Storing a reference to the Channel in the Client - e.g. client._channel.
  2. Returning the channel from the ClientBuilder. e.g. client, channel = builder.construct().

client._channel is more ergonomic/friendly/breaks nothing. Returning from the builder feels more "correct" regarding how gRPC wants to operate though - but would be a breaking code change.

Going with both - a reference in the Client for the sake of preserving behavior, and adding another method ClientBuilder.construct_with_channel.

miott commented 4 years ago

Don't see a point in returning the channel separate from the Client instance.

remingtonc commented 4 years ago

If I had better thought about this in the past, I would have initially returned the Channel as well as the Client instance. Call it wishful thinking. When gNOI and other gRPC based interfaces start coming into play we will likely want to preserve our channel instances as they'll likely be shared.