You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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().
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.
The text was updated successfully, but these errors were encountered:
mickyhale
changed the title
Request timeout handling is confusing
Request timeout is not honored
Feb 5, 2024
I was attempting to adjust request timeout to 40 which I believed would affect all call methods in
CvpApi
, but it wasn't working forget_configlets_and_mappers()
.Debug snippet from my app:
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 inCvpClient
(instead ofCvpApi
). It could happen either in the connect call whereconnect_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.
The text was updated successfully, but these errors were encountered: