feat(datasource-pylon): foundation — gem, config & resilient client - #341
Merged
christophebrun-forest merged 4 commits intoAug 7, 2026
Conversation
Story 1 of the Pylon datasource (EXT-5). Adds the forest_admin_datasource_pylon gem skeleton: Zeitwerk autoloading, typed error hierarchy with an APIError carrying HTTP status and parsed body, configurable logger, Configuration with api_key validation, and a Faraday client authenticating with a Bearer token plus a GET /me health check. The Faraday middleware order is deliberate and differs from the Mambu Payments gem: raise_error sits outside the JSON parser so errors carry an already-parsed body, and retry sits innermost so it can observe raw statuses. Behind raise_error the retry middleware never sees a 429 and retry_statuses silently does nothing. Non-idempotent verbs are only retried on 429, where Pylon rejected the request before processing it. Wires the package into the CI lint, test and coverage jobs. The semantic-release publish pipeline is intentionally left untouched until Story 9, so an incomplete gem is never pushed to RubyGems. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
3 new issues
|
![qltysh[bot]](https://avatars.githubusercontent.com/in/890766?s=60&v=4){kind=small}
| gem 'rspec', '~> 3.0' | ||
| gem 'simplecov', '~> 0.22', require: false | ||
| gem 'webmock', '~> 3.0' | ||
| end |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 Medium
agent-ruby/.github/workflows/build.yml
Line 133 in 8d94a91
The Send coverage step references reports/3.4-forest_admin_datasource_pylon/coverage.json, but coverage artifacts are uploaded only for ruby-version == '4.0', so the artifact is named 4.0-forest_admin_datasource_pylon. The 3.4 path does not exist, so the Pylon coverage file is missing and the qltysh/qlty-action/coverage step receives a non-existent input. The coverage job matrix uses 3.4, which mismatches the 4.0 upload condition. Either use 4.0 in the files path for Pylon or align the upload and coverage job matrices to the same version.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @.github/workflows/build.yml around line 133:
The `Send coverage` step references `reports/3.4-forest_admin_datasource_pylon/coverage.json`, but coverage artifacts are uploaded only for `ruby-version == '4.0'`, so the artifact is named `4.0-forest_admin_datasource_pylon`. The `3.4` path does not exist, so the Pylon coverage file is missing and the `qltysh/qlty-action/coverage` step receives a non-existent input. The coverage job matrix uses `3.4`, which mismatches the `4.0` upload condition. Either use `4.0` in the `files` path for Pylon or align the upload and coverage job matrices to the same version.
Comment on lines
+32
to
+33
| spec.add_dependency 'activesupport', '>= 6.1' | ||
| spec.add_dependency 'faraday', '~> 2.0' |
There was a problem hiding this comment.
🟠 High forest_admin_datasource_pylon/forest_admin_datasource_pylon.gemspec:32
The gemspec omits forest_admin_datasource_toolkit from its runtime dependencies, so installing forest_admin_datasource_pylon does not pull in the toolkit. When users require 'forest_admin_datasource_pylon', the gem fails with LoadError because the toolkit gem is missing. Add forest_admin_datasource_toolkit as a runtime dependency via spec.add_dependency.
spec.add_dependency 'forest_admin_datasource_toolkit'
+ spec.add_dependency 'activesupport', '>= 6.1'🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/forest_admin_datasource_pylon/forest_admin_datasource_pylon.gemspec around lines 32-33:
The gemspec omits `forest_admin_datasource_toolkit` from its runtime dependencies, so installing `forest_admin_datasource_pylon` does not pull in the toolkit. When users `require 'forest_admin_datasource_pylon'`, the gem fails with `LoadError` because the toolkit gem is missing. Add `forest_admin_datasource_toolkit` as a runtime dependency via `spec.add_dependency`.
Review findings on EXT-5. max_interval was hardcoded to 5s. faraday-retry gives up outright when Retry-After exceeds max_interval, so a Pylon 429 carrying a per-minute Retry-After performed zero retries -- defeating the retry the story is about, on the very endpoints Story 2 will hit (10 req/min). Verified: Retry-After 30 issued 1 request, no retry. The cap now defaults to 65s to cover a full rate-limit window and is configurable. Three specs pin it: the give-up mechanism when the cap is exceeded, a header within the cap still retrying, and the default staying above a per-minute window. The existing specs missed this because none of them sent a Retry-After header. Also from the review: - retry Faraday::ConnectionFailed, absent from faraday-retry's defaults, so transient connection drops are absorbed too - truncate the error message before appending the request_id, which a long error body used to push out first - require json explicitly rather than via Faraday - drop a dead compact in error_detail Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
|
||
| Logger.new($stderr).tap { |l| l.progname = 'forest_admin_datasource_pylon' } | ||
| end | ||
| end |
Gathers everything governing how the client reacts to a failed request into one value object: retryable statuses and exceptions, the idempotent-verb rule and its retry_if escape hatch for 429s, the budget and the Retry-After cap. The client no longer spreads that policy across four constants and three Configuration kwargs; it just splats to_faraday_options. Configuration drops from 7 keyword arguments to 5, so the Metrics/ParameterLists exclusion added for it is no longer needed and is removed rather than left to rot. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
|
||
| attr_reader :api_key, :base_url, :open_timeout, :timeout, :retry_policy | ||
|
|
||
| def initialize(api_key:, base_url: nil, open_timeout: 5, timeout: 30, retry_policy: RetryPolicy.new) |
activesupport was declared as a runtime dependency but nothing in the package uses it: blank? is hand-rolled in Configuration. Declaring it forced a heavy transitive dependency on every consumer for nothing. must_succeed rescued StandardError after Faraday::Error, so an APIError raised inside the block was caught by the generic arm and re-wrapped, dropping its status to nil. Nothing does that yet, but the collections landing next branch on status == 404, so the failure mode has teeth. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment on lines
+53
to
+55
| nested = parsed['error'] | ||
| message = parsed['message'] || (nested.is_a?(Hash) ? nested['message'] : nested) || | ||
| join_errors(parsed['errors']) |
There was a problem hiding this comment.
🟡 Medium forest_admin_datasource_pylon/client.rb:53
An API error payload like { "message": "", "errors": [{ "message": "invalid field" }] } surfaces the serialized whole payload instead of the validation message from errors. parsed['message'] is an empty string, which is truthy in Ruby, so error_message keeps the empty value and append_request_id falls through to parsed.to_json — bypassing join_errors(parsed['errors']) which would have returned the real error text. Use presence (or a blank check) so empty-string candidates are treated as absent and the fallback chain proceeds.
- nested = parsed['error']
- message = parsed['message'] || (nested.is_a?(Hash) ? nested['message'] : nested) ||
- join_errors(parsed['errors'])
+ nested = parsed['error']
+ message = parsed['message'].presence ||
+ (nested.is_a?(Hash) ? nested['message'].presence : nested&.presence) ||
+ join_errors(parsed['errors'])🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/forest_admin_datasource_pylon/lib/forest_admin_datasource_pylon/client.rb around lines 53-55:
An API error payload like `{ "message": "", "errors": [{ "message": "invalid field" }] }` surfaces the serialized whole payload instead of the validation message from `errors`. `parsed['message']` is an empty string, which is truthy in Ruby, so `error_message` keeps the empty value and `append_request_id` falls through to `parsed.to_json` — bypassing `join_errors(parsed['errors'])` which would have returned the real error text. Use `presence` (or a blank check) so empty-string candidates are treated as absent and the fallback chain proceeds.
christophebrun-forest
deleted the
feature/ext-5-story-1-foundation-gem-config-resilient-client
branch
August 7, 2026 16:24
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.
Story 1 of the Pylon datasource — EXT-5. Targets the integration branch
feat/datasource-pylon(EXT-4), notmain.What this adds
A new
forest_admin_datasource_pylongem skeleton with an authenticated, rate-limit-aware HTTP client:for_gemautoloading, typed error hierarchy (Error / ConfigurationError / UnsupportedOperatorError / APIError), configurable logger with aRails.loggerfallback.APIErrorcarries the HTTPstatusand the parsedbodyso the smart actions in Story 8 can surface Pylon's own validation message.Configuration— validatedapi_key, base URLhttps://api.usepylon.com(Pylon paths are unversioned), configurable timeouts and retry budget.Client— Faraday withAuthorization: Bearer, JSON in/out, 429 retry with exponential backoff,GET /mehealth check, plus the envelope-unwrapping and error-mapping helpers the later stories build on.Faraday middleware order — deliberate divergence from the Mambu Payments gem
forest_admin_datasource_mambu_paymentsregistersretrybeforeraise_error, which means 429s are never actually retried:raise_errorsits inside, so it raisesFaraday::TooManyRequestsError, which is not in faraday-retry's defaultexceptions— the middleware never observes a response andretry_statusessilently does nothing.This gem inverts the order:
raise_erroroutside the JSON parser (errors carry an already-parsed body) andretryinnermost, where it sees raw statuses. Two specs pin the behaviour — one 429 then 200 issues 2 requests; a persistent 429 issues 3 and raisesAPIErrorwithstatus: 429.Non-idempotent verbs are retried only on 429, where Pylon rejected the request before processing it — a 502 on
POST /issuesmay well have created the issue. Implemented withretry_ifrather thanmethods, because faraday-retry ORs the two conditions andretry_ifcan therefore only widen, never restrict.Worth a separate fix on the Mambu gem.
Monorepo wiring
forest_admin_datasource_pylonadded to thelintmatrix,testmatrix andcoveragefile list inbuild.yml, plus the needed.rubocop.ymlexcludes..releaserc.jsis intentionally untouched, so no incomplete gem can reach RubyGems — thedeployjob only runs on push tomain/beta, so no story PR into the integration branch can publish. The exact 3-line patch is recorded as a comment on EXT-13 (Story 9), which owns "add gem to the release/publish workflow". Consequence: the PylonVERSIONstays at1.36.2and will drift frommainuntil Story 9 realigns it.Test plan
BUNDLE_GEMFILE=Gemfile-test bundle exec rspec→ 29 examples, 0 failures, coverage 96.97% (threshold 90)bundle exec rubocopover the whole repo → 786 files, no offensesClient/Configuration🤖 Generated with Claude Code
Note
Add
forest_admin_datasource_pylongem with configuration and resilient HTTP clientforest_admin_datasource_pylonRuby gem with a Zeitwerk-loaded module, structuredAPIError(carrying HTTP status and parsed body), and a logger that defaults toRails.loggerwhen available.Configurationrequiringapi_key, defaultingbase_urltohttps://api.usepylon.com, and bundling aRetryPolicy.RetryPolicyretries on 429/502/503/504 and connection failures with exponential backoff, capping at 65s to respect PylonRetry-Afterheaders; non-idempotent verbs only retry on 429.Clientwraps Faraday with JSON middleware and the retry policy, maps failures toAPIErrorwithrequest_idpropagation and message truncation.Macroscope summarized f07ce77.