Skip to content

Conversation

jamesbraza
Copy link
Collaborator

Before: aiohttp and httpx coexisted in this repo

After: we just have httpx

@jamesbraza jamesbraza self-assigned this Aug 11, 2025
@jamesbraza jamesbraza added the enhancement New feature or request label Aug 11, 2025
@Copilot Copilot AI review requested due to automatic review settings August 11, 2025 23:19
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Aug 11, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR consolidates HTTP client libraries by replacing aiohttp with httpx throughout the repository to achieve a single, unified HTTP client approach.

  • Replace aiohttp.ClientSession with httpx.AsyncClient across all test files
  • Update exception handling from aiohttp.ClientResponseError to httpx.HTTPStatusError
  • Refresh test cassettes with new HTTP request headers and formatting changes from the httpx client

Reviewed Changes

Copilot reviewed 41 out of 51 changed files in this pull request and generated no comments.

File Description
tests/test_clinical_trials.py Updates test fixtures and mocks to use httpx instead of aiohttp for clinical trials API testing
tests/test_clients.py Replaces aiohttp.ClientSession with httpx.AsyncClient in metadata client tests and updates citation count test data
tests/cassettes/*.yaml Updates VCR cassettes with new request headers and response formatting from httpx instead of aiohttp

@jamesbraza jamesbraza force-pushed the httpx-migration branch 4 times, most recently from e04ddb9 to 0d51fd6 Compare August 12, 2025 21:47
@whitead
Copy link
Collaborator

whitead commented Aug 15, 2025

Why the rename? Won't that break backwards compatibility?

@jamesbraza
Copy link
Collaborator Author

Why the rename? Won't that break backwards compatibility?

You mean rename from session to client? Yeah this PR is breaking in two ways:

  1. The actual type is incompatible (aiohttp.ClientSession vs httpx.AsyncClient)
  2. The name is incompatible (session vs client)

I thought the name change was good because:

  • NameError is safer than silent aiohttp.ClientSession | None mismatches with None fallback
  • It resolves the name collision between paperqa.PQASession and atiohttp.ClientSession

Copy link
Collaborator

@whitead whitead left a comment

Choose a reason for hiding this comment

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

Nice

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Aug 16, 2025
@jamesbraza jamesbraza force-pushed the httpx-migration branch 2 times, most recently from cbf01a2 to d383e5a Compare August 22, 2025 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants