Skip to content

Conversation

treagod
Copy link
Contributor

@treagod treagod commented Apr 16, 2025

Summary

This PR refactors the Http class to remove its persistent HTTP::Client usage and instead delegate the creation of HTTP clients to a new factory interface (HttpClientFactory). By doing so, each request can optionally acquire a client from a custom provide.

What Changed

  1. Removed the single @client field in Http:
  • Previously, Http stored one HTTP::Client instance and reused it for all requests.
  • Now, Http does not store any long-lived client. Instead, it calls a factory’s #acquire_client method on every request.
  1. Introduced HttpClientFactory and DefaultHttpClientFactory:
  • HttpClientFactory is an abstract class with three key methods:
    • #acquire_client(endpoint, signer) – returns a fully configured HTTP::Client.
    • #acquire_raw_client(endpoint, signer) – returns a plainHTTP::Client, must be implemented by developer.
    • #release(client) – allows returning or cleaning up the client. (Default implementation is no-op.)
  • DefaultHttpClientFactory provides the current “create a new client each time” logic, so no immediate behavioral change occurs unless users swap in a different factory.
  1. New request lifecycle in Http:
  • Each HTTP request (e.g. delete, get, head, etc.) uses acquire_client to get a ready-to-use client.
  • After the request finishes (or fails), Http calls release(client) to allow the factory to clean up or recycle resources.

How the New Mechanism Works

  1. Http obtains an HTTP::Client on demand:
client = @factory.acquire_client(@endpoint, @signer)
  1. Request Execution:
  • The #exec method sends the HTTP request using the acquired client.
  • Upon success or failure, we exit the loop and proceed to the ensure block.
  1. Release:
  • @factory.release(client) is called, allowing the factory to decide whether to close, reuse, or discard the client.

Closes #77

Copy link
Collaborator

@miry miry left a comment

Choose a reason for hiding this comment

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

I left a few comments.

@treagod
Copy link
Contributor Author

treagod commented Apr 20, 2025

@miry I addressed all of your findings 👍🏻

@miry miry merged commit 97dc354 into taylorfinnell:master Apr 21, 2025
6 checks passed
@miry
Copy link
Collaborator

miry commented Apr 21, 2025

@treagod Thank you very much for all this work!

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.

Connection pooling and socket errors
2 participants