Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

📝 Add HTTP/2+ use case into the advanced usage section #1111

Merged
merged 11 commits into from Jan 3, 2024

Conversation

Ousret
Copy link
Contributor

@Ousret Ousret commented Dec 7, 2023

That PR addresses a question from the community regarding the evolution of the HTTP backend. Still, It's a workaround, but it's a start.

Niquests is a drop-in replacement for Requests that offers an escape hatch for hvac users who desire to leverage recent HTTP capabilities. The documentation now showcases two workarounds for the limitations imposed by Requests.

  • Using Niquests's Session directly
  • Create a hvac adapter around Niquests

Linked to #807 (comment)

This alternative HTTP backend brings a lot of enhancement, I am sure that we'll find some synergies.

@Ousret Ousret requested a review from a team as a code owner December 7, 2023 04:48
@briantist briantist self-assigned this Dec 10, 2023
@briantist briantist added documentation documentation updates and/or requests for expanded documentation patch Used as part of release-drafter's version-resolver configuration adapters related to the Adapter system client related to the hvac Client labels Dec 10, 2023
Copy link

codecov bot commented Dec 10, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (26bb929) 87.38% compared to head (98a8492) 87.38%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1111   +/-   ##
=======================================
  Coverage   87.38%   87.38%           
=======================================
  Files          66       66           
  Lines        3227     3227           
=======================================
  Hits         2820     2820           
  Misses        407      407           

@Ousret
Copy link
Contributor Author

Ousret commented Jan 1, 2024

Hello again,

FYI you could be potentially interested in knowing that the newest release of Niquests support secure/encrypted DNS with DNSSEC, amongst other things.
Due to the critical nature of this project in terms of security, this is worth mentioning, so I think.

import niquests

s = niquests.Session(resolver="doh+google://", multiplexed=True)
...

Do you want a note in the docs about it?

@briantist briantist added this to the 2.1.0 milestone Jan 1, 2024
@briantist
Copy link
Contributor

Sure feel free to add that to this PR!
I hope to review it soon, I will probably edit it slightly to maybe add a note or change sections around so that there's a place for this kind of unofficial things, just so we can be clear that it's user-contributed and can't necessarily be supported by hvac's maintainers. I'll think about how exactly to do group that.

Niquests is a drop-in replacement for Requests that offer an escape hatch for hvac user that desire to leverage recent HTTP capabilities.
@briantist
Copy link
Contributor

Ok! So, I created a whole new top-level section for user-contributed use cases like these, and yours will be the first. I loved your doc into there.

I made some other minor changes:

  • (first, updated your branch with changes from main)
  • changed a little formatting
  • added/updated some links
  • updated the contributing guide to better describe how to build docs locally

If you'd like to tweak any the wording or the links, please go ahead. I chose your project's "issues" link as the "support" link but if there's a better place, please update the link.

There's only one other tweak I'd like to ask you to do, and that's change the multiplexing code sample to not have to scroll horizontally, if possible. Maybe it's only on my screen? Not sure, please take a look when you can.

You can render the documentation locally by following the instructions in the contributor guide (use some of the steps added in this PR 😅).


One other request: unless you really need or want to, please do not squash commits locally. We squash-and-merge all of our PRs so the individual commits won't matter on the merge.

Squashing locally makes it a little harder for me to review; I'd rather be able to see the individual changes, especially as PR development goes on.

If you must squash locally, the PR will still be reviewed it just may take longer :)

@briantist briantist removed this from the 2.1.0 milestone Jan 2, 2024
@Ousret
Copy link
Contributor Author

Ousret commented Jan 3, 2024

I am okay with your changes.
Also did what I can to reduce horizontal scrolling for smaller screen.

@briantist briantist added this to the 2.1.0 milestone Jan 3, 2024
Copy link
Contributor

@briantist briantist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks so much for this contribution @Ousret !

@briantist briantist merged commit 5b3c161 into hvac:main Jan 3, 2024
35 checks passed
@Ousret Ousret deleted the patch-docs branch January 3, 2024 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adapters related to the Adapter system client related to the hvac Client documentation documentation updates and/or requests for expanded documentation patch Used as part of release-drafter's version-resolver configuration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants