chore(rest-api): Clean up past API deprecations, add Fern docs for deprecation#2316
chore(rest-api): Clean up past API deprecations, add Fern docs for deprecation#2316thossain-nv wants to merge 3 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
thossain-nv
force-pushed
the
chore/openapi-cleanup
branch
from
12fe132 to
c78c1e1
Compare
|
🌿 Preview your docs: https://nvidia-preview-pull-request-2316.docs.buildwithfern.com/infra-controller |
🔐 TruffleHog Secret Scan✅ No secrets or credentials found! Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉 🕐 Last updated: 2026-06-08 22:33:08 UTC | Commit: 12fe132 |
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
rest-api/openapi/spec.yaml (2)
249-260:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: SDK and documentation must be regenerated after OpenAPI spec changes.
The pipeline failure indicates that
rest-api/openapi/spec.yamlwas modified but the corresponding SDK (rest-api/sdk/standard/) and documentation (rest-api/docs/index.html) were not regenerated. This violates the coding guideline requirement to ensure generated artifacts remain synchronized with the spec.Run the SDK generation command to resolve the pipeline failure:
#!/bin/bash # Description: Regenerate SDK and documentation from the updated OpenAPI spec cd rest-api make generate-sdk make publish-openapi🤖 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 `@rest-api/openapi/spec.yaml` around lines 249 - 260, The OpenAPI spec change in rest-api/openapi/spec.yaml requires regenerating the SDK and docs so generated artifacts match the spec; run the SDK/doc generation from the rest-api directory (e.g., invoke the project targets used in CI such as make generate-sdk and make publish-openapi), verify that rest-api/sdk/standard/ and rest-api/docs/index.html are updated, then add/commit those generated files alongside the spec change so the pipeline passes.Source: Coding guidelines
249-260:⚠️ Potential issue | 🟠 MajorUpdate getting-started.md deprecation navigation to match the centralized “Deprecations” structure
rest-api/openapi/spec.yamlandrest-api/openapi/deprecations.mdcentralize deprecations under “Deprecations” / “Active Deprecations” / “Recent Deprecations”, butrest-api/openapi/getting-started.mdstill tells users to find deprecation notices by “Click on each API resource…”, which will lead them to the wrong location (or nowhere) with the new structure.
- Change the sentence in the “API Version” section of
rest-api/openapi/getting-started.mdto point users to the centralized “Deprecations” section (e.g., the Active/Recent blocks), consistent with the “Deprecations (#tag/Deprecations)” reference already present in the OpenAPI spec.🤖 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 `@rest-api/openapi/spec.yaml` around lines 249 - 260, In the "API Version" section of rest-api/openapi/getting-started.md locate the sentence that tells users to "Click on each API resource…" and replace it with a line pointing to the centralized Deprecations section in the OpenAPI spec (referencing "Deprecations" and its subsections "Active Deprecations" / "Recent Deprecations"); ensure the new sentence mirrors the spec's wording (e.g., "See the centralized Deprecations section (Active Deprecations / Recent Deprecations) in the OpenAPI spec for breaking-change notices") so it matches the structure used in rest-api/openapi/spec.yaml and rest-api/openapi/deprecations.md.Source: Coding guidelines
🧹 Nitpick comments (4)
fern/README.md (2)
29-33: ⚡ Quick winAdd context for the SQLite experimental flag.
The troubleshooting entry omits why Fern docs requires
node:sqliteand when users will encounter this (e.g., Node.js version constraints, dependency on an experimental module). Readers debugging this error will benefit from understanding whether it affects all Node.js 22.x installations, only certain minor versions, or is tied to a specific Fern CLI behavior.📝 Proposed expansion
If the command fails with `No such built-in module: node:sqlite`, set `NODE_OPTIONS` before running `fern docs dev`: ```bash export NODE_OPTIONS="--experimental-sqlite"Note: This flag is required for Node.js versions where the
node:sqlitemodule is experimental. Fern's documentation tooling uses SQLite for [brief explanation of what component needs it, e.g., "local content indexing" or "dependency graph caching"].</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@fern/README.mdaround lines 29 - 33, Expand the troubleshooting entry for
No such built-in module: node:sqliteto explain why the flag is needed and
when users will see it: mention thenode:sqlitebuilt-in is experimental in
some Node.js 22.x releases (so certain minor/patch versions require the flag),
state that Fern's docs tooling (thefern docs devcommand) uses SQLite for
local content indexing/dependency-graph caching, and keep the existing fix
showing to set NODE_OPTIONS="--experimental-sqlite" before runningfern docs dev; reference the terms node:sqlite, NODE_OPTIONS, and fern docs dev in the
updated copy.</details> <!-- cr-comment:v1:8efd843470e404f4a586e8e5 --> --- `29-29`: _⚡ Quick win_ **Clarify the environment variable reference.** The phrase "set this env var" is ambiguous—readers must scan ahead to discover you mean `NODE_OPTIONS`. State it explicitly: "set `NODE_OPTIONS` before running…" removes the pronoun lookup and makes the instruction immediately actionable. <details> <summary>📝 Proposed revision</summary> ```diff -If the command fails with `No such built-in module: node:sqlite`, set this env var before running `fern docs dev`: +If the command fails with `No such built-in module: node:sqlite`, set `NODE_OPTIONS` before running `fern docs dev`:🤖 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 `@fern/README.md` at line 29, Replace the ambiguous phrase "set this env var" in the sentence that begins "If the command fails with `No such built-in module: node:sqlite`" with an explicit reference to the environment variable by changing it to "set the NODE_OPTIONS environment variable" (or "set `NODE_OPTIONS`") so the instruction reads clearly: "If the command fails with `No such built-in module: node:sqlite`, set the NODE_OPTIONS environment variable before running `fern docs dev`."rest-api/openapi/deprecations.md (1)
3-3: 💤 Low valueMinor phrasing refinement.
"maintains backward compatibility with the previous versions" reads slightly awkwardly. Consider "maintains backward compatibility with prior versions" for clearer flow.
🤖 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 `@rest-api/openapi/deprecations.md` at line 3, Update the phrasing of the sentence in rest-api/openapi/deprecations.md that currently reads "NICo REST API maintains backward compatibility with the previous versions." to instead read "NICo REST API maintains backward compatibility with prior versions." — locate the exact sentence string in that file and replace it to improve flow and clarity.rest-api/openapi/spec.yaml (1)
112-112: 💤 Low valueMinor: Inconsistent capitalization of "deprecations" in section headings.
Line 112 uses "Recent deprecations:" (lowercase 'd'), while lines 252 and 256 use "Active Deprecations:" and "Recent Deprecations:" (uppercase 'D'). Consistent heading style improves readability and professionalism of the API documentation.
Proposed fix for consistent capitalization
- Recent deprecations: + Recent Deprecations:Alternatively, adopt lowercase throughout if that better matches the OpenAPI specification's style guide.
Also applies to: 252-256
🤖 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 `@rest-api/openapi/spec.yaml` at line 112, Standardize the capitalization of the deprecation section headings in the OpenAPI spec by making the "Recent deprecations:", "Active Deprecations:" and "Recent Deprecations:" headings consistent; locate the heading strings (e.g., "Recent deprecations:" and "Active Deprecations:") in the spec.yaml and change them all to a single style (either "Recent Deprecations:" and "Active Deprecations:" Title Case or both lowercase like "recent deprecations:"/ "active deprecations:") and update every occurrence so the headings match across the file.
🤖 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 `@rest-api/openapi/deprecations.md`:
- Around line 63-68: The guidance is missing the post-expiry lifecycle for
deprecated request attributes (referencing takeActionBy); add a new section that
states: after the takeActionBy date the deprecated attribute must no longer
appear in API representations (it may remain in the database for backward
compatibility but must be omitted from responses), any incoming create/update
requests that include the deprecated attribute continue to return HTTP 400, and
consumers should map to the new attribute; update the bullets in the “If the
deprecated item is an attribute” block to reference this post-expiry behavior
explicitly and include an example sentence describing that the field may persist
in storage but will not be returned by API endpoints.
- Around line 50-61: The JSON deprecation example uses incorrect field names;
replace "queryParam" with "queryparam", "replacedBy" with "replacedby", and
"takeActionBy" with "effective" in the deprecations entry (and make the same
three-key changes in the attribute and endpoint deprecation examples) so the
example keys match the implementation.
- Around line 11-22: The JSON field names in the docs (replacedBy, takeActionBy)
don't match the Go model's JSON tags; update the Go struct in
rest-api/api/pkg/api/model/deprecation.go by changing the JSON tags for the
ReplacedBy field to "replacedBy" and for the TakeActionBy field to
"takeActionBy" (ensure the struct names ReplacedBy and TakeActionBy remain
unchanged), run tests and regenerate any affected API clients/docs, and
coordinate this breaking-change roll-out with consumers; alternatively, if you
opt not to break clients, instead update rest-api/openapi/deprecations.md to use
the current wire names "replacedby" and "effective" to match the existing tags.
- Line 42: The sentence is ambiguous about whether `replacedBy` is omitted or
present with null; update the sentence to match the Go model (the struct field
`ReplacedBy *string` with JSON tag "replacedby" has no `omitempty`) and state
explicitly that the wire format will include the field with a null value (i.e.,
`"replacedby": null`) when there is no replacement; reference the `ReplacedBy
*string` field and the `"replacedby"` JSON tag so readers can locate the
implementation.
- Around line 30-40: The documentation uses the JSON field "takeActionBy" but
the API serializes this value as "effective"; update all deprecation examples in
this file (including the shown endpoint object and the other two deprecation
type examples) to replace "takeActionBy" with "effective" so the documented JSON
keys match the actual API serialization and consumers can parse the deprecation
payload correctly.
---
Outside diff comments:
In `@rest-api/openapi/spec.yaml`:
- Around line 249-260: The OpenAPI spec change in rest-api/openapi/spec.yaml
requires regenerating the SDK and docs so generated artifacts match the spec;
run the SDK/doc generation from the rest-api directory (e.g., invoke the project
targets used in CI such as make generate-sdk and make publish-openapi), verify
that rest-api/sdk/standard/ and rest-api/docs/index.html are updated, then
add/commit those generated files alongside the spec change so the pipeline
passes.
- Around line 249-260: In the "API Version" section of
rest-api/openapi/getting-started.md locate the sentence that tells users to
"Click on each API resource…" and replace it with a line pointing to the
centralized Deprecations section in the OpenAPI spec (referencing "Deprecations"
and its subsections "Active Deprecations" / "Recent Deprecations"); ensure the
new sentence mirrors the spec's wording (e.g., "See the centralized Deprecations
section (Active Deprecations / Recent Deprecations) in the OpenAPI spec for
breaking-change notices") so it matches the structure used in
rest-api/openapi/spec.yaml and rest-api/openapi/deprecations.md.
---
Nitpick comments:
In `@fern/README.md`:
- Around line 29-33: Expand the troubleshooting entry for `No such built-in
module: node:sqlite` to explain why the flag is needed and when users will see
it: mention the `node:sqlite` built-in is experimental in some Node.js 22.x
releases (so certain minor/patch versions require the flag), state that Fern's
docs tooling (the `fern docs dev` command) uses SQLite for local content
indexing/dependency-graph caching, and keep the existing fix showing to set
NODE_OPTIONS="--experimental-sqlite" before running `fern docs dev`; reference
the terms node:sqlite, NODE_OPTIONS, and fern docs dev in the updated copy.
- Line 29: Replace the ambiguous phrase "set this env var" in the sentence that
begins "If the command fails with `No such built-in module: node:sqlite`" with
an explicit reference to the environment variable by changing it to "set the
NODE_OPTIONS environment variable" (or "set `NODE_OPTIONS`") so the instruction
reads clearly: "If the command fails with `No such built-in module:
node:sqlite`, set the NODE_OPTIONS environment variable before running `fern
docs dev`."
In `@rest-api/openapi/deprecations.md`:
- Line 3: Update the phrasing of the sentence in
rest-api/openapi/deprecations.md that currently reads "NICo REST API maintains
backward compatibility with the previous versions." to instead read "NICo REST
API maintains backward compatibility with prior versions." — locate the exact
sentence string in that file and replace it to improve flow and clarity.
In `@rest-api/openapi/spec.yaml`:
- Line 112: Standardize the capitalization of the deprecation section headings
in the OpenAPI spec by making the "Recent deprecations:", "Active Deprecations:"
and "Recent Deprecations:" headings consistent; locate the heading strings
(e.g., "Recent deprecations:" and "Active Deprecations:") in the spec.yaml and
change them all to a single style (either "Recent Deprecations:" and "Active
Deprecations:" Title Case or both lowercase like "recent deprecations:"/ "active
deprecations:") and update every occurrence so the headings match across the
file.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2154086a-1116-4628-84cd-09458162dcf4
📒 Files selected for processing (11)
docs/index.ymldocs/openapi/spec.yamlfern/README.mdrest-api/api/pkg/api/model/instance.gorest-api/api/pkg/api/model/machine.gorest-api/api/pkg/api/model/machine_test.gorest-api/docs/index.htmlrest-api/openapi/deprecations.mdrest-api/openapi/getting-started.mdrest-api/openapi/spec.yamlrest-api/sdk/standard/model_tenant_identity_config_create_or_update_request.go
💤 Files with no reviewable changes (3)
- rest-api/api/pkg/api/model/machine_test.go
- rest-api/sdk/standard/model_tenant_identity_config_create_or_update_request.go
- rest-api/api/pkg/api/model/instance.go
| ```json | ||
| { | ||
| "deprecations": [ | ||
| { | ||
| "attribute": "displayName", | ||
| "replacedBy": "orgDisplayName", | ||
| "takeActionBy": "2026-06-08T00:00:00Z", | ||
| "notice": "`displayName` has been deprecated in favor of `orgDisplayName`. Please take action prior to the specified date" | ||
| } | ||
| ] | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Critical: JSON field names do not match implementation.
The documentation examples use camelCase field names ("replacedBy", "takeActionBy"), but the Go model in rest-api/api/pkg/api/model/deprecation.go serializes these as "replacedby" and "effective" respectively. Clients following this documentation will fail to parse API responses correctly.
The implementation shows:
ReplacedBy *stringwith JSON tag"replacedby"TakeActionBy time.Timewith JSON tag"effective"
Either update this documentation to match the actual wire format, or update the Go struct JSON tags to match these documented field names. The latter is preferable for API clarity, but requires coordination as it's a breaking change for existing clients.
🤖 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 `@rest-api/openapi/deprecations.md` around lines 11 - 22, The JSON field names
in the docs (replacedBy, takeActionBy) don't match the Go model's JSON tags;
update the Go struct in rest-api/api/pkg/api/model/deprecation.go by changing
the JSON tags for the ReplacedBy field to "replacedBy" and for the TakeActionBy
field to "takeActionBy" (ensure the struct names ReplacedBy and TakeActionBy
remain unchanged), run tests and regenerate any affected API clients/docs, and
coordinate this breaking-change roll-out with consumers; alternatively, if you
opt not to break clients, instead update rest-api/openapi/deprecations.md to use
the current wire names "replacedby" and "effective" to match the existing tags.
| ```json | ||
| { | ||
| "deprecations": [ | ||
| { | ||
| "endpoint": "POST /org/:orgName/nico/infrastructure-provider", | ||
| "takeActionBy": "2026-06-08T00:00:00Z", | ||
| "notice": "`POST /org/:orgName/nico/infrastructure-provider` has been deprecated. Please take action prior to the specified date" | ||
| } | ||
| ] | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Critical: Same field name mismatch for endpoint deprecations.
The "takeActionBy" field name documented here does not match the actual API serialization key "effective". This issue affects all three deprecation types documented in this file.
🤖 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 `@rest-api/openapi/deprecations.md` around lines 30 - 40, The documentation
uses the JSON field "takeActionBy" but the API serializes this value as
"effective"; update all deprecation examples in this file (including the shown
endpoint object and the other two deprecation type examples) to replace
"takeActionBy" with "effective" so the documented JSON keys match the actual API
serialization and consumers can parse the deprecation payload correctly.
b9c9576 to
6f2f8a1
Compare
thossain-nv
force-pushed
the
chore/openapi-cleanup
branch
from
6f2f8a1 to
3cc42d0
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
thossain-nv
force-pushed
the
chore/openapi-cleanup
branch
from
3cc42d0 to
dfaf495
Compare
thossain-nv
force-pushed
the
chore/openapi-cleanup
branch
4 times, most recently
from
429dde5 to
ce5941e
Compare
thossain-nv
force-pushed
the
chore/openapi-cleanup
branch
from
ce5941e to
a161406
Compare
…precation Signed-off-by: Tareque Hossain <thossain@nvidia.com>
Signed-off-by: Tareque Hossain <thossain@nvidia.com>
Signed-off-by: Tareque Hossain <thossain@nvidia.com>
thossain-nv
force-pushed
the
chore/openapi-cleanup
branch
from
a161406 to
05d1ee2
Compare
3 participants
Description
Type of Change
Breaking Changes
Testing