Add timeout parameter to API object

[skatsaounis]

Solves #166

Signed-off-by: Stamatis Katsaounis [email protected]

[janw]

Thanks you for this. It is long overdue.

[skatsaounis]

Hi @janw I can see that there are conflicts with the current master branch. I will try to resolve them and update this PR. Plus, I will also take into account the comment on #187

[skatsaounis]

Hi @zachmoody if you have a couple of minutes to spare please review this PR and let me know if it is valid or if it misses anything

[skatsaounis]

@zachmoody Hi, if you think this PR is of no benefit, I would like to close it instead of re-resolving conflicts. Please let me know because I am doing housekeeping to my GitHub.

Kind regards,
Stamatis

[zachmoody]

Hi @skatsaounis, thanks for the PR. There's definitely benefit to it, I'm just slightly hesitant to merge because I keep thinking (in the little time I do get to spend on this project) there's a more comprehensive approach we can take to expose these knobs in requests people need. If you don't mind keeping it open, we might still use it, but certainly don't feel like it's necessary to keep coming back and resolving conflicts. I'll take care of those if we merge.

[skatsaounis]

Thank you for letting me know. There is no problem to leave this PR open, especially now that I am aware of what is happening. By the way, thanks for this extremely useful python client.

[markkuleinio]

I'll toss my 0.05€ in... As I see it based on https://requests.readthedocs.io/en/master/_modules/requests/sessions/, the timeout value is not Session-specific but it needs to be specified separately for each request (which kind of makes sense). Thus, it seems we need to carry the timeout value separately to be used with each request within pynetbox calls, and giving it as an argument to .Api() (and saving it in the instance for further use) seems reasonable. Unless there is a vision that the timeout value should be different in different NetBox API calls of course...

Edit: And now I stumbled into https://hodovi.ch/blog/advanced-usage-python-requests-timeouts-retries-hooks/#setting-default-timeouts that uses adapters with Session to specify a default timeout value. So, if Session override is added (#229), users can also implement timeouts at their will.

[zachmoody]

Edit: And now I stumbled into https://hodovi.ch/blog/advanced-usage-python-requests-timeouts-retries-hooks/#setting-default-timeouts that uses adapters with Session to specify a default timeout value. So, if Session override is added (#229), users can also implement timeouts at their will.

💯 Came across something similar in the FRs for requests. I think it's the way to go, but working through the best way to expose it. Initial feeling is that this may be a bit much to ask users to do in order to set something like timeouts, but maybe I'm wrong.

[zachmoody]

This was implemented in v5.0. Closing out this PR.