Add default retry to hvac client requests

[notatallshaw-gts]

This implements a retry mechanism to the hvac client as the libraries documentation suggests.

I am providing this due to a real-world case of an HTTP Connection Error causing a task to initially fail to start, after investigating I found the hvac documentation suggested adding this retry process.

Due to the docker version I have access to and the minimum requirements of Airflow dev tooling I hit limitations in the MyPy pre-commits and running the test suite, but I believe this PR should pass all tests.

Let me know if you need any additional work and/or changes to the PR to be able to review and/or land.

Finally, someone else submitted a similar PR but it is failing due import errors and they have not updated the PR since it was created three weeks ago: #30628

---

^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

[notatallshaw-gts]

Ah, the session is causing the call asserts to fail.

I guess I can fix them by adding an expected session=client.kwargs['session'] but maybe there's a more elegant solution, I will think about.

[hussein-awala]

IMO this needs to be tested, even if it looks trivial. We should avoid the possibility of someone accidentally breaking it. You can add two tests, one with the session argument and one without.

And for the failed tests, since there is no argument to activate/deactivate the retry (we can do this by providing a None session), you should fix all of them. I think we can replace the mock_hvac.Client.assert_called_with by getting the call args and compare the urls, otherwise you need to get the session from the kwargs and compare it to the passed session, which is useless.

[notatallshaw-gts]

IMO this needs to be tested, even if it looks trivial. We should avoid the possibility of someone accidentally breaking it. You can add two tests, one with the session argument and one without.

Agreed, I'll create some new tests (fyi it may take me a few days to update this when I have the time to work on it)

And for the failed tests, since there is no argument to activate/deactivate the retry (we can do this by providing a None session), you should fix all of them.

Sure I can do this

I think we can replace the mock_hvac.Client.assert_called_with by getting the call args and compare the urls, otherwise you need to get the session from the kwargs and compare it to the passed session, which is useless.

I'm not sure I follow, if I fix the tests by passing session=None as stated above what do you want to do with a url?

[potiuk]

LGTM. @hussein-awala @kaxil ?

[notatallshaw-gts]

I haven't had chance to add new tests yet, I will try and add them tomorrow or Monday

[potiuk]

conflicts to resolve

[notatallshaw-gts]

conflicts to resolve

Thanks for the heads up, I will fix.

Apologies for not keeping to my earlier estimate of providing tests. I will look to get this PR ready as soon as I can.

[potiuk]

Needs conflict resolving

[notatallshaw-gts]

Needs conflict resolving

Resolved conflict and added new test to check default retry is added to hvac client. Thanks for your time, let me know if anything else needs to be done to merge.

[notatallshaw-gts]

@hussein-awala @potiuk Anything further needed to do for this to be merged?

[potiuk]

@hussein-awala @potiuk Anything further needed to do for this to be merged?

Atttention grabbing of those who could merge it (which BTW. you just did).