HYPERFLEET-1272 - feat: rename query string params#8
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAPI and service versions were bumped to 1.0.25. List endpoints for channels, channel versions, clusters, cluster nodepools, nodepools, and wifconfigs now use Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
rh-amarin
force-pushed
the
HYPERFLEET-1272
branch
from
842d2a8 to
00bb8b2
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@schemas/template/openapi.yaml`:
- Around line 31-33: The OpenAPI QueryParams contract is being changed from
pageSize to size before the rest of the stack is ready, which will break
generated clients and the live service. Update the shared QueryParams source to
keep pageSize as a deprecated alias alongside size, and align the API
implementation, integration tests, and Sentinel client references
(QueryParamsPageSize, PageSize) before regenerating schemas/template and related
generated artifacts. Do not edit generated files directly; make the contract
change in the source definitions that feed generation so all consumers stay in
sync.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: c978727e-66ba-4b8a-9980-2c4be28fac9b
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
main.tspschemas/template/openapi.yamlschemas/template/swagger.yaml
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
| - $ref: '#/components/parameters/QueryParams.page' | ||
| - $ref: '#/components/parameters/QueryParams.pageSize' | ||
| - $ref: '#/components/parameters/QueryParams.orderBy' | ||
| - $ref: '#/components/parameters/QueryParams.size' | ||
| - $ref: '#/components/parameters/QueryParams.order' |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
pageSize → size is an uncoordinated contract break.
The linked API implementation and clients are not on this contract yet: openshift-hyperfleet/hyperfleet-api/pkg/services/types.go still prefers pageSize, its integration tests still call QueryParamsPageSize(...), and openshift-hyperfleet/hyperfleet-sentinel/internal/client/client.go still emits PageSize. Publishing only size here will desync generated clients and the running service. Keep a deprecated pageSize alias in the shared QueryParams source until the API, Sentinel, and generated clients land together, then regenerate schemas/template/*.
As per coding guidelines, "DO NOT modify generated files in schemas/ or tsp-output-template/ directly." As per path instructions, "Validate changes against HyperFleet architecture standards from the linked architecture repository."
Also applies to: 240-242, 463-465, 781-783, 1122-1124, 1160-1163, 1378-1387
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@schemas/template/openapi.yaml` around lines 31 - 33, The OpenAPI QueryParams
contract is being changed from pageSize to size before the rest of the stack is
ready, which will break generated clients and the live service. Update the
shared QueryParams source to keep pageSize as a deprecated alias alongside size,
and align the API implementation, integration tests, and Sentinel client
references (QueryParamsPageSize, PageSize) before regenerating schemas/template
and related generated artifacts. Do not edit generated files directly; make the
contract change in the source definitions that feed generation so all consumers
stay in sync.
Sources: Coding guidelines, Path instructions, Linked repositories
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
schemas/template/openapi.yaml (1)
31-33: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
pageSize→sizeis an uncoordinated contract break (CWE-16). Edited directly in a generated artifact.Two problems persist:
- Consumers not aligned.
hyperfleet-api/pkg/services/types.gostill preferspageSize(and its tests assert that precedence),hyperfleet-sentinel/internal/client/client.gostill emitsPageSize, and integration suites still callopenapi.QueryParamsPageSize(...)/QueryParamsOrderBy(...). Publishingsize-only (and droppingorderBy) here desyncs generated clients from the live service. Land a deprecatedpageSizealias in the sharedQueryParamssource until API, Sentinel, and generated clients cut over together.- Wrong edit surface. This is a generated file; the rename and the
orderBy/OrderDirectionremoval must be made in the TypeSpec source and regenerated, not hand-edited here.As per coding guidelines: "DO NOT modify generated files in
schemas/ortsp-output-template/directly." As per path instructions: "Validate changes against HyperFleet architecture standards from the linked architecture repository."Also applies to: 240-242, 463-465, 781-783, 1122-1124, 1160-1163, 1378-1387
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@schemas/template/openapi.yaml` around lines 31 - 33, The OpenAPI contract change was made in a generated artifact instead of the source, and it breaks consumers still relying on QueryParams.pageSize and QueryParamsOrderBy. Move the rename/removal into the TypeSpec source that generates QueryParams, keep a deprecated pageSize alias until hyperfleet-api/pkg/services/types.go, hyperfleet-sentinel/internal/client/client.go, and the integration tests are updated together, and then regenerate schemas/openapi.yaml rather than editing it directly.Sources: Coding guidelines, Path instructions, Linked repositories
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Line 10: The hyperfleet core spec dependency has been bumped to a contract
version that switches list pagination from pageSize to size, but downstream
clients in this repo still build and parse pageSize. Revert or defer the
package.json hyperfleet version change until the generated contract and all list
clients are updated together, and keep the dependency pinned to the last
compatible spec version referenced by the hyperfleet package entry.
In `@schemas/template/openapi.yaml`:
- Line 1544: The OpenAPI change making request-body `kind` required looks like
accidental generated drift, not a scoped manual edit. Update the TypeSpec source
definitions for the affected POST bodies (channel, cluster, nodepool, version,
and wifconfig) to mark `kind` as required/optional correctly, then regenerate
the OpenAPI YAML instead of editing `schemas/template/openapi.yaml` directly.
Also confirm this breaking contract change is intentional, aligned with the
`pkg/api/*_types.go` and handler behavior, and recorded in the changelog.
---
Duplicate comments:
In `@schemas/template/openapi.yaml`:
- Around line 31-33: The OpenAPI contract change was made in a generated
artifact instead of the source, and it breaks consumers still relying on
QueryParams.pageSize and QueryParamsOrderBy. Move the rename/removal into the
TypeSpec source that generates QueryParams, keep a deprecated pageSize alias
until hyperfleet-api/pkg/services/types.go,
hyperfleet-sentinel/internal/client/client.go, and the integration tests are
updated together, and then regenerate schemas/openapi.yaml rather than editing
it directly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 9712c3ed-aac9-4eb9-9cbc-00d835040a88
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
main.tsppackage.jsonschemas/template/openapi.yamlschemas/template/swagger.yaml
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
🚧 Files skipped from review as they are similar to previous changes (2)
- main.tsp
- schemas/template/swagger.yaml
| }, | ||
| "dependencies": { | ||
| "hyperfleet": "github:openshift-hyperfleet/hyperfleet-api-spec#main" | ||
| "hyperfleet": "github:openshift-hyperfleet/hyperfleet-api-spec#v1.0.24" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== package.json ==\n'
cat -n package.json | sed -n '1,40p'
printf '\n== search for pageSize / size in spec files ==\n'
rg -n '"pageSize"|pageSize|orderBy|size' . --glob '!node_modules' --glob '!schemas/**' --glob '!tsp-output-template/**' || true
printf '\n== main.tsp / changelog references ==\n'
rg -n '`@info`|v1\.0\.24|v1\.0\.25|pageSize|size' main.tsp CHANGELOG.md schemas/template/openapi.yaml 2>/dev/null || trueRepository: openshift-hyperfleet/hyperfleet-api-spec-template
Length of output: 2863
Hold this core spec bump until downstream list clients switch to size (package.json:10). hyperfleet-api and hyperfleet-sentinel still build/parse pageSize, while this repo now publishes size in the generated contract. Shipping v1.0.24 here creates a breaking pagination mismatch for list endpoints.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` at line 10, The hyperfleet core spec dependency has been bumped
to a contract version that switches list pagination from pageSize to size, but
downstream clients in this repo still build and parse pageSize. Revert or defer
the package.json hyperfleet version change until the generated contract and all
list clients are updated together, and keep the dependency pinned to the last
compatible spec version referenced by the hyperfleet package entry.
Sources: Path instructions, Linked repositories
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rafabene The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
Bot
merged commit 15e51e6
into
openshift-hyperfleet:main
2 participants
Summary
This is to align with spec version 1.0.24
Changed the dependency in
package.jsonto point to a tag in the hyperfleet-api-spec repository instead of main