search

aristanetworks / cvprac

Other
46 stars 47 forks source link

Request timeout is not honored #265

Open mickyhale opened 7 months ago

mickyhale commented 7 months ago

I was attempting to adjust request timeout to 40 which I believed would affect all call methods in CvpApi, but it wasn't working for get_configlets_and_mappers().

Debug snippet from my app:

...
[cvprac] get_configlets_and_mappers: getConfigletsAndAssociatedMappers
[cvprac] HTTPSConnectionPool(host='some.host', port=443): Read timed out. (read timeout=30)
...

I took a look at the code and saw that timeout wasn't set for that call. That would be a simple enough fix but then I looked at how request_timeout is being passed and it seems like it should be an attribute set in CvpClient (instead of CvpApi). It could happen either in the connect call where connect_timeout is set or during instantiation (exposing both timeouts to the user prior to connect might be nice).

I was going to submit a pull request with the proposed change, but thought I might bring it up first in case I'm missing the reasoning behind how it's configured and because the change that I'm suggesting isn't entirely trivial.

Thank you for the library.

mharista commented 6 months ago

Thanks @mickyhale

I'll take a look at this and try to get it addressed before the next release.