adapters: if session is user-supplied, do not overwrite session options with Client/Adapter options

[Tylerlhess]

Fixes: #991

Breaking change

For more details see also:

• Breaking changes coming to Adapters' use of custom sessions #1040

Before this change:

• if you do not use a custom session, a session is created for you with values for the verify, cert, and proxies settings that are set in the adapter, either explicitly or by default
• if you do use a custom session:
  • the values in the session will always be overwritten with the adapter values, whether you specified them or not

After this change:

• if you do not use a custom session, a session is created for you with values for the verify, cert, and proxies settings that are set in the adapter, either explicitly or by default
• if you do use a custom session:
  • if any value is set for the verify, cert, or proxies properties in your custom session, then that value will be kept and the adapter's value will be ignored, even if you set it on the adapter explicitly
  • if any of those properties are not set then they will be set based on the adapter's value for those

The only case where this should cause breakage of existing code is where all of the following are true:

• you use a custom session
• you set any of the affected properties on the custom session
• the value(s) you set on those properties are actually invalid for your use case, and so your code relied on the adapter defaults overriding your values

[codecov]

Codecov Report

Merging #1021 (3e04a1b) into main (6837bf0) will increase coverage by 0.06%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1021      +/-   ##
==========================================
+ Coverage   85.02%   85.08%   +0.06%     
==========================================
  Files          65       65              
  Lines        3139     3145       +6     
==========================================
+ Hits         2669     2676       +7     
+ Misses        470      469       -1     

Files Changed	Coverage Δ
hvac/adapters.py	90.35% <100.00%> (+0.53%)	⬆️

... and 1 file with indirect coverage changes

[briantist]

Hey @Tylerlhess thanks for this. I haven't looked at it fully yet, but I wanted to mention I opened #1023 to try to deal with our out-of-date Vault versions in the integration tests.

My hope is we can get all the issues ironed out in there so that you don't have to add these unrelated changes into your PR. I've already borrowed your azure fix (with attribution) in b01e796 though since we're also not pinning to X.Y.0 in that PR anymore, it seems like some changes may have made it into patch versions of Vault, so we may need to be more nuanced in the version compares now.

I really appreciate you bringing these issues to our attention!

[briantist]

With #1023 & #1024 merged, those should fix the Azure failures, SSH failures you saw locally, and the coverage issues, so you can rebase against or merge from the latest main branch HEAD, and remove your azure changes.

I was going to do this for you and push up the branch but for some reason it looks a little weird to me, when I try to rebase your branch all of the files are "conflicts" even the ones you didn't touch. Trying to merge main into your branch refuses due to "unrelated histories" which is probably the same reason for rebase weirdness.

I think I can resolve that anyway and force push, so if you want me to go ahead with that please let me know, otherwise I'll leave it you.

Issue resolved, was on my end.

---

The other changes in the PR look good to me, the only other thing I'd like to do is add warnings for users who are affected, telling them that in a future major version we'll stop applying any overrides from the affected parameters when a session is supplied.

I can also add that stuff in alongside the work you've done already.

[Tylerlhess]

Looks like those changes have resolved the issues that would be happening in 1.12 and 1.13.