akarneliuk / pygnmi

The pure Python implementation of the gNMI client.
https://training.karneliuk.com
BSD 3-Clause "New" or "Revised" License
127 stars 44 forks source link

grpc keep alive feature not working #53

Closed andsouth44 closed 2 years ago

andsouth44 commented 2 years ago

In line 67/68 of client.py, I think it should read:

if 'keepalive_time_ms' in kwargs: self.configureKeepalive(**kwargs)

Otherwise:

TypeError: configureKeepalive() got an unexpected keyword argument 'interval_ms'

akarneliuk commented 2 years ago

Hello @andsouth44 ,

thanks for reporting. Could you please share a code snippet, where you face that? We were adding the unit tests for this functionality, so I wonder if we missed something.

Best, Anton

andsouth44 commented 2 years ago

I create an instance of gnmi client class like this with 'interval_ms' kwarg set so that a grpc keepalive is requested.

gnmi_client = gNMIclient(target=("1.1.1.1", "50051"),
                    username="username",
                    password="password",
                    path_cert="cert",
                    path_key="key",
                    path_root="root",
                    override="test",
                    interval_ms=1000)

The 'interval_ms' kwarg is recognized by the client code at line 67:

67        if 'interval_ms' in kwargs:
68            self.configureKeepalive(**kwargs)

The client code passes the the 'interval_ms' kwarg to the configureKeepalive method at line 68:

    def configureKeepalive(self, keepalive_time_ms: int, keepalive_timeout_ms: int = 20000,
                           max_pings_without_data: int = 0,
                           keepalive_permit_without_calls: bool = True):

The configureKeepalive method is not looking for an 'interval_ms' argument so I get this error:

TypeError: configureKeepalive() got an unexpected keyword argument 'interval_ms'

If I change line 67/68 of client to this:

  67      if 'keepalive_time_ms' in kwargs:
  68         self.configureKeepalive(**kwargs)

And create my client like this:

gnmi_client = gNMIclient(target=("1.1.1.1", "50051"),
                    username="username",
                    password="password",
                    path_cert="cert",
                    path_key="key",
                    path_root="root",
                    override="test",
                    keepalive_time_ms=1000)

It works ok.

akarneliuk commented 2 years ago

Understood, thanks. will push release in a few.

akarneliuk commented 2 years ago

Hello @andsouth44 ,

Just pushed a new release. Could you please check if that is OK, now?

Thanks, Anton

andsouth44 commented 2 years ago

Hi Anton, Yes. That looks good.

Thanks Andy