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

v6.0.0 Replace Requests with HTTPX #145

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

Deconstrained
Copy link
Collaborator

@Deconstrained Deconstrained commented Nov 16, 2024

Justification

Long-term, we are looking to add support for thread safety, i.e. to fulfill #132. This requires switching the HTTP client on which pdpyras is based to one that is thread-safe. HTTPX is thread-safe and provides an interface that is very similar to that of Requests, and so minimal changes will be necessary in order to adopt it. Moreover, it optionally supports HTTP/2 which gives us the option of supporting that protocol in the future as well. Finally, allegations regarding how the maintainers of Requests have responded to responsible security disclosures give us pause, and compel us to switch out the base HTTP client for the benefit of PagerDuty customers' security.

Summary of Changes

  • Replace requests.Session with httpx.Client and requests.Response with httpx.Response
  • Deprecate the "prepare_headers" method, which was not needed and complicated integration with HTTPX
  • Add a new "default_headers" property to take its place

To-do

  • Validate that sending GET requests with content-type: application/json won't mess with how query parameters are interpreted and thus that it is safe to proceed with the simplification that pays off a bit of tech debt
  • Enumerate all the possible exception classes that could be raised by httpx and its dependencies and update the error handling logic to recognize them
  • Write down the full list of deprecations and breaking changes (if any)
  • Test the various functions of the new client using a live key
  • Add a basic integration test to CircleCI (could be any single Python version, it needn't be all of them) using the same API key as publicly given for demo purposes in the developer documentation

* Deprecate the "prepare_headers" method
* Add a new "default_headers" property to take its place (this simplifies the developer interface)
@Deconstrained Deconstrained force-pushed the v6.0.0-replace-requests branch from aedc91c to 19405f4 Compare November 20, 2024 17:04
@Deconstrained
Copy link
Collaborator Author

Note on test failures: the second to-do item above should take care of them, because the library currently makes the assumption that the local environment has urllib3 by virtue of installing requests (it's a second-removed upstream dependency).

@Deconstrained Deconstrained marked this pull request as ready for review December 17, 2024 20:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant