Reuse a Faraday client per Trino::Client instance

[p623]

Purpose

Reuse a single Faraday HTTP client for each Trino::Client instance.

Currently, query operations create a new Faraday client for each request.
Reusing the client reduces connection and TLS handshake overhead and enables HTTP keep-alive with a compatible Faraday adapter.

This PR is based on the idea and implementation proposed in #137. Since that PR has not been updated for over a year, this PR provides a fresh implementation for the current codebase with expanded test coverage.

Thank you to @redox for the original proposal and implementation.

Overview

• Reuse one Faraday client per Trino::Client instance
• Set query-specific HTTP headers for each request
• Preserve the existing behavior of query operations
• Add extensive tests for client reuse and request-specific behavior
• Update the implementation for the current codebase

Checklist

• Code compiles correctly
• Created tests which fail without the change (if possible)
• All tests passing
• Extended the README / documentation, if necessary

[yuokada]

Sorry for the late response.

One question: Could you clarify why Basic Auth was moved from Faraday’s middleware to manually generated per-request headers? Was this needed for connection reuse or compatibility with Faraday 1 and 2?

[p623]

Thanks for taking a look!

This wasn’t required for connection reuse or Faraday 1/2 compatibility.
The goal was to preserve the previous behavior of using the credentials from options at query start, rather than fixing them at client initialization.
Basic Auth is now generated alongside the other query-specific headers and reused throughout that query, without modifying the shared Faraday connection.

[yuokada]

Thanks for your explanation. One edge case: if a client is created with HTTP and no password, then a password is added to the options before a query; #168 reuses the HTTP connection but adds the credentials to the Authorization header.

On master, each query creates a Faraday connection using the current options. During setup, the client checks that password authentication uses HTTPS before configuring the Basic Auth middleware. In short, master checks the credentials and connection scheme together at query time, while #168 checks the scheme only when the Client is initialized. Could we recheck that the connection is HTTPS before adding Basic Auth to each query?

[p623]

Thanks for pointing this out.
I’ve added an HTTPS check against the actual shared connection before generating Basic Auth headers, along with regression tests.
While looking into this, I also realized that connection reuse changes how some options are handled: master picks up changes to connection options such as ssl at query start, whereas #168 applies them only at client initialization.

I propose keeping connection settings fixed for the lifetime of the client, with a new client required to change them. Query-specific headers, including Basic Auth, would still use the options at query start.
I’ve documented this behavior in the README’s options list. Would this approach be acceptable?

[p623]

Thanks for the review and approval, @yuokada!
I don’t have merge permissions for this repository.
Could you merge this when you have a chance, or let me know if anything else is needed from my side?

[yuokada]

I need a bit of time to check the impact on our internal repos. I'll follow up here once I finish checking.