fix(storage): pin distinct operationIds for iceberg's analytics-bucket routes - #1274
fix(storage): pin distinct operationIds for iceberg's analytics-bucket routes#1274grdsdev wants to merge 1 commit into
Conversation
…t routes iceberg/bucket.ts reuses ROUTE_OPERATIONS.CREATE_BUCKET/DELETE_BUCKET/ LIST_BUCKET verbatim - the same constants src/http/routes/bucket/*.ts uses for the unrelated S3 storage-bucket endpoints. Two entirely different resources (S3 storage buckets vs Iceberg analytics buckets) deriving the same OpenAPI operationId. Pins createIcebergBucket/ deleteIcebergBucket/listIcebergBuckets explicitly so the generated SDK gets distinct, correctly-named methods for each.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — straightforward operationId pinning that follows the pattern established in #1270.
Extended reasoning...
Overview
This PR adds explicit operationId values (createIcebergBucket, deleteIcebergBucket, listIcebergBuckets) to the three routes in src/http/routes/iceberg/bucket.ts, to avoid colliding with the identically-named ROUTE_OPERATIONS constants used by the unrelated S3 storage-bucket routes. It is a three-line, additive change to route config blocks only — no request handling, auth, or business logic is touched.
Security risks
None. The change only affects OpenAPI documentation metadata (the generated operationId used for SDK method naming); it has no effect on runtime request handling, authorization, or data access.
Level of scrutiny
Low. This is a mechanical doc-generation fix stacked on the config.operationId override mechanism introduced in #1270 (verified in src/http/routes/openapi-transform.ts:173, where route.config?.operationId takes precedence over the derived id). The mechanism itself is already covered by existing tests in openapi-transform.test.ts (e.g. "prefers an explicit config.operationId over the derived one"), so this PR is just applying an already-tested capability to three routes.
Other factors
The PR is intentionally scoped to a single file and includes a clear before/after rationale tied to a specific review discussion on #1270. Test plan reports passing full unit suite, clean typecheck, and clean lint. No outstanding review comments in the timeline.
Coverage Report for CI Build 30383438639Warning No base build found for commit Coverage: 80.478%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
2 participants
Context
Follow-up from #1270 review discussion - stacked on top of it since it depends on the
config.operationIdmechanism added there. Rebase onto master once #1270 merges.Summary
src/http/routes/iceberg/bucket.tsreusesROUTE_OPERATIONS.CREATE_BUCKET/DELETE_BUCKET/LIST_BUCKETverbatim - the exact same constantssrc/http/routes/bucket/*.tsuses for the unrelated S3 storage-bucket endpoints. Two entirely different resources (S3 storage buckets vs Iceberg analytics buckets) were deriving the same OpenAPIoperationId.Pins
createIcebergBucket/deleteIcebergBucket/listIcebergBucketsexplicitly viaconfig.operationId, so the generated SDK gets distinct, correctly-named methods for each instead of the collision being silently papered over (or, with #1270's original stricter design, crashing doc generation - see #1270's own fix for that).Test plan
tsc -noEmit— no errorsbiome checkon touched file — clean