[SPIKE] PAT-1866 Symfony HttpClient instead of Guzzle (base + git-service)#517
Draft
pepamartinec wants to merge 5 commits into
Draft
[SPIKE] PAT-1866 Symfony HttpClient instead of Guzzle (base + git-service)#517pepamartinec wants to merge 5 commits into
pepamartinec wants to merge 5 commits into
Conversation
Replace the duplicated transport/auth/retry/JSON plumbing (git-service was
the reference the base lib was extracted from) with the shared base. The
GitServiceApiClient facade takes the Manage API token + tunable options as
flat constructor params; a null token defaults to service-account auth (the
projected SA token), preserving the original default behavior. The
{code, error} error format is a standalone GitServiceErrorMessageResolver.
Replace the Guzzle transport with symfony/http-client: - ApiClient wraps HttpClientInterface; requests issued as (method, path, options) instead of PSR-7 Request objects. - Auth contract becomes a header map (getAuthenticationHeaders()); AuthenticatingHttpClient decorator merges them per request, wrapped inside RetryableHttpClient so auth re-resolves on every retry. - Retry handled by Symfony GenericRetryStrategy via RetryStrategyFactory (transport errors + all 5xx + configured codes, for all methods). - Drop Json + RetryDecider; use response toArray()/getContent() and the json request option. DefaultErrorMessageResolver decodes the body itself. - ApiClientOptions swaps the Guzzle requestHandler for an injectable HttpClientInterface test seam. - Tests rewritten on MockHttpClient/MockResponse.
- Swap guzzlehttp/guzzle + psr/http-message for symfony/http-client in composer.json; the base lib now provides the Symfony transport. - Facade methods call sendRequest/sendRequestAndMapResponse with (method, path, options) instead of building PSR-7 Request objects; JSON bodies pass via the 'json' request option. - Constructor test seam becomes ?HttpClientInterface $httpClient (was the Guzzle requestHandler); auth logic unchanged. - GitServiceErrorMessageResolver json_decodes the body directly (Json helper removed from the base lib). - Tests rewritten on MockHttpClient/MockResponse.
…body only on error)
pepamartinec
force-pushed
the
pepa/common-api-lib-git-service
branch
3 times, most recently
from
f90cd5b to
f6662fe
Compare
pepamartinec
force-pushed
the
pepa/common-api-lib-git-service
branch
from
f6662fe to
93bf863
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Throwaway exploration for PAT-1866: what would adopting Symfony HttpClient instead of Guzzle do for
php-api-client-base?https://linear.app/keboola/issue/PAT-1866/unified-api-client-base
This branch is based on the git-service migration branch (
pepa/common-api-lib-git-service, PR #516) on purpose, so the diff below is exactly the Guzzle → Symfony HttpClient swap for the base lib + git-service — the side-by-side comparison. The Guzzle versions to compare against are PRs #513 (base) and #516 (git-service).Both libs are
composer cigreen on Symfony HttpClient (base 34 tests, git-service 45 tests, PHPStan level max with zero@phpstan-ignoreinsrc/). Full writeup:libs/php-api-client-base/SPIKE_NOTES.md.What Symfony HttpClient lets us delete
RetryDecider(67 LOC) →RetryableHttpClient+ a 37-LOC strategy factory (backoff/jitter/attempt-counting/retry-logging free).Json.php→$response->toArray()+['json' => …]request bodies.RetryableHttpClientlogs retries.guzzlehttp/guzzleandpsr/http-message.MockHttpClient/MockResponsetesting (records the request;getRequestsCount()for retry asserts).What changes (breaking — absorbed in the base once, then re-migrate the fleet)
getAuthenticationHeaders(): array, applied by anAuthenticatingHttpClientdecorator wrapped insideRetryableHttpClient(so auth re-runs per retry attempt — covered by a test).Request→(method, path, options)(facade methods became one-liners).requestHandler→ inject a?HttpClientInterface(MockHttpClient).getContent()to surface errors); Symfony default retry is idempotent-only (we pass a flat 5xx list to match old behavior).Why it's interesting for us
Keboola services run on Symfony, so they'd get an autowired
HttpClientInterface,framework.http_client.scoped_clientsconfig, and profiler/TraceableHttpClientobservability for free. The flat-scalar facade DI shape is unaffected by the choice — only the internal authenticator contract + request-building change.Bottom line
Worth adopting for new clients and Symfony-hosted services, but it's an API + contract break — not a drop-in swap. See
SPIKE_NOTES.mdfor the full recommendation.