feat(dsx): provider implementation - #452
Conversation
|
/ok-to-test b462f7d |
Greptile SummaryThe PR implements and registers the DSX topology provider, including authenticated HTTP access, paginated topology retrieval, topology conversion, simulation support, tests, and provider documentation.
Confidence Score: 4/5The PR is not yet safe to merge because retryable DSX failures can still recreate the ten-minute generation deadline and keep one generation active for roughly 50 minutes. The current code makes deadline expiry terminal, but an API error returned just before expiry remains a retryable 502; the server then invokes the provider again and creates another full generation deadline. Files Needing Attention: pkg/providers/dsx/instance_topology.go, pkg/server/engine.go Important Files Changed
Sequence DiagramsequenceDiagram
participant Server
participant DSXProvider
participant DSXClient
participant DSXAPI
Server->>DSXProvider: GenerateTopologyConfig
loop Until empty token or termination guard
DSXProvider->>DSXClient: GetTopology(genCtx, token)
DSXClient->>DSXAPI: GET /v1/topology/nodes
DSXAPI-->>DSXClient: switches + next_page_token
DSXClient-->>DSXProvider: topology page
end
DSXProvider->>DSXProvider: Resolve cross-page ancestry
DSXProvider-->>Server: Canonical topology graph
Reviews (18): Last reviewed commit: "feat(dsx): provider implementation" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #452 +/- ##
==========================================
+ Coverage 72.15% 76.87% +4.71%
==========================================
Files 89 97 +8
Lines 5689 7053 +1364
==========================================
+ Hits 4105 5422 +1317
+ Misses 1382 1350 -32
- Partials 202 281 +79 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (4)
📜 Recent review details⏰ Context from checks skipped due to timeout. (1)
🧰 Additional context used📓 Path-based instructions (5)**/*.go📄 CodeRabbit inference engine (AGENTS.md)
Files:
pkg/providers/**/*.go📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*📄 CodeRabbit inference engine (AGENTS.md)
Files:
{cmd,pkg,internal}/**/*.go⚙️ CodeRabbit configuration file
Files:
**/*_test.go📄 CodeRabbit inference engine (AGENTS.md)
Files:
⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (5)
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesThe PR adds the DSX topology provider. It implements configuration loading, authenticated HTTP retrieval, pagination, topology construction, simulation support, registry registration, tests, and documentation. DSX provider integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant generateInstanceTopology
participant httpClient
participant DSX_API
participant buildClusterTopology
generateInstanceTopology->>httpClient: GetTopology(page token)
httpClient->>DSX_API: Send topology request
DSX_API-->>httpClient: Return topology page and next_page_token
httpClient-->>generateInstanceTopology: Decode topology response
generateInstanceTopology->>httpClient: Request subsequent pages
generateInstanceTopology->>buildClusterTopology: Pass accumulated switch data
buildClusterTopology-->>generateInstanceTopology: Return instance topologies
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![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 `@docs/providers/dsx.md`:
- Line 5: Align the provider documentation with the implementation: remove the
explicit per-VPC endpoint guidance and describe only the global GET
/v1/topology/nodes request, including that it aggregates
topology.ComputeInstances across regions without using Region or a VPC ID.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: a2bce70a-5a15-4992-b38e-92f7d0127b0f
📒 Files selected for processing (10)
docs/overview.mddocs/providers/dsx.mdpkg/providers/dsx/client.gopkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.gopkg/providers/dsx/provider.gopkg/providers/dsx/provider_sim.gopkg/providers/dsx/provider_sim_test.gopkg/providers/dsx/provider_test.gopkg/registry/registry.go
|
🌿 Preview your docs: https://nvidia-preview-pull-request-452.docs.buildwithfern.com/topograph |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@docs/providers/dsx.md`:
- Line 130: Update the pagination description near the graph-building
explanation to state that switch entries are accumulated across pages, and the
topology graph is built only after the final page is fetched, when cross-page
ancestry is resolved. Remove the claim that each page is merged before
requesting the next page.
In `@pkg/providers/dsx/client.go`:
- Around line 54-93: Add direct httptest-based regression tests that construct
the real client through NewHTTPClient and call GetTopology, covering both VPC
and node URL paths, query parameter encoding, conditional Authorization headers,
non-2xx responses, and malformed JSON in an otherwise successful response. Keep
the existing mockClient tests unchanged and assert the resulting responses or
errors for each scenario.
In `@pkg/providers/dsx/instance_topology_test.go`:
- Around line 85-199: Extend the topology test table with malformed-input cases:
add a cyclic switch relationship and assert topology construction terminates
without looping, and add repeated occurrences of the same NodeID and assert that
requested instances are emitted only once. Anchor both cases to the existing
topology-building test flow and preserve the current want/wantLen validation
pattern.
🪄 Autofix
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: ASSERTIVE
Plan: Enterprise
Run ID: f1a02019-1f1b-4693-9d8c-cf2fc4ef81f1
📒 Files selected for processing (3)
docs/providers/dsx.mdpkg/providers/dsx/client.gopkg/providers/dsx/instance_topology_test.go
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (6)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Rungo fmt ./...; Go formatting is authoritative and code should not be hand-formatted.
Every new Go file must include the NVIDIA copyright header followed by the matching Apache 2.0 boilerplate.
Files:
pkg/providers/dsx/client.gopkg/providers/dsx/instance_topology_test.go
pkg/providers/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
pkg/providers/**/*.go: Providers must return a*topology.Graphand*httperr.Error; plainerroris not acceptable at the provider interface boundary.
Providers discover topology and return the canonicaltopology.Graph; they must not emit scheduler-specific output.
When usingClusterTopology, populate fabric tiers closest-first, preserve optional accelerator domain and sub-domain fields, and callToGraph.
Files:
pkg/providers/dsx/client.gopkg/providers/dsx/instance_topology_test.go
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Do not publicly disclose suspected security vulnerabilities; report them privately through NVIDIA PSIRT using the channels documented inSECURITY.md.
Every commit must include aSigned-off-by:trailer for DCO compliance.
Use Conventional Commits with an allowed type such asfeat,fix,docs,chore,refactor,test,build, orci.
Before pushing, runmake qualify; all CI checks, including Go build/test/lint, Codecov, and DCO, must be green before merge.
Changes to provider, engine, chart, API, configuration schema, labels, annotations, or user-facing behavior must include the corresponding documentation updates described in the documentation impact table.
Files:
pkg/providers/dsx/client.godocs/providers/dsx.mdpkg/providers/dsx/instance_topology_test.go
{cmd,pkg,internal}/**/*.go
⚙️ CodeRabbit configuration file
{cmd,pkg,internal}/**/*.go: Focus on correctness, robustness, and failure paths:
- Check error handling and propagation, including whether callers
receive enough context to diagnose failures.- Verify context propagation, cancellation, timeouts, resource cleanup,
and goroutine lifecycle.- Look for races, deadlocks, unsafe shared state, leaks, and partial
updates that can leave state inconsistent.- Check nil, empty, malformed, duplicate, boundary, and partial inputs.
- Verify deterministic and idempotent behavior where operations may be
retried or repeated.- Preserve public contracts and the architectural boundaries documented
in AGENTS.md.- Flag behavior changes that lack meaningful regression coverage.
Files:
pkg/providers/dsx/client.gopkg/providers/dsx/instance_topology_test.go
docs/providers/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Provider additions or changes require the corresponding provider documentation, including prerequisites, credentials, parameters, operation, and verification.
Files:
docs/providers/dsx.md
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
New or changed public behavior and new code paths should be covered by tests.
Files:
pkg/providers/dsx/instance_topology_test.go
⚙️ CodeRabbit configuration file
**/*_test.go: Flag missing negative, cancellation, concurrency, malformed-input,
boundary, and regression cases. Check that assertions would fail if
the behavior under test were broken.
Files:
pkg/providers/dsx/instance_topology_test.go
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@pkg/providers/dsx/client_test.go`:
- Around line 64-179: Add a caller-cancellation case to
TestHTTPClientGetTopology using a blocking httptest.Server handler that signals
request receipt, then cancel the context and release or observe the blocked
request. Invoke GetTopology with that context and assert it returns an error
with a nil response promptly, without waiting for requestTimeout, while
preserving the existing table-driven coverage.
- Around line 183-186: Update the query-parameter assertions in the test loop
over tc.wantQuery: when want is empty, assert cap.query does not contain param
instead of using url.Values.Get; retain require.Equal for non-empty expected
values. Ensure the empty nodeIDs and zero pageSize cases fail if the client
sends those parameters with empty values.
In `@pkg/providers/dsx/instance_topology_test.go`:
- Around line 38-50: Extend mockClient.GetTopology to record each request’s
pageSize and pageToken, then update
TestGenerateInstanceTopologyCrossPageAncestry to assert tokens ["", "page2"] and
verify a configured page size is forwarded unchanged on every call. Ensure the
assertions inspect the recorded arguments so they fail if
generateInstanceTopology stops propagating the response token or configured page
size.
In `@pkg/providers/dsx/instance_topology.go`:
- Around line 89-93: Validate the successful result from Client.GetTopology
before accessing response.Switches: when response is nil and apiErr is nil,
return an httperr.NewError with HTTP 502 instead of dereferencing it. Add a
regression test using a client that returns nil, nil and verify the gateway
error is returned.
- Around line 48-61: After building nodeIDs in the topology-fetch flow, return
topology.NewClusterTopology() immediately when nodeIDs is empty, without calling
httpClient.GetTopology. Update the affected test in
pkg/providers/dsx/instance_topology_test.go:448-463 to expect an empty topology,
zero client calls, and mc.idx == 0; the production change is in
pkg/providers/dsx/instance_topology.go:48-61.
- Around line 53-60: Update the node ID collection near want and nodeIDs so each
instance ID is appended only when first inserted into want, preventing
duplicates across cis entries. After collecting all IDs, sort nodeIDs before the
first GetTopology call to ensure deterministic requests.
🪄 Autofix
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: ASSERTIVE
Plan: Enterprise
Run ID: c4754ea9-f326-443d-8e67-4c959475b0ec
📒 Files selected for processing (4)
docs/providers/dsx.mdpkg/providers/dsx/client_test.gopkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Rungo fmt ./...; Go formatting is authoritative and code should not be hand-formatted.
Every new Go file must include the NVIDIA copyright header followed by the matching Apache 2.0 boilerplate.
Files:
pkg/providers/dsx/client_test.gopkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
pkg/providers/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
pkg/providers/**/*.go: Providers must return a*topology.Graphand*httperr.Error; plainerroris not acceptable at the provider interface boundary.
Providers discover topology and return the canonicaltopology.Graph; they must not emit scheduler-specific output.
When usingClusterTopology, populate fabric tiers closest-first, preserve optional accelerator domain and sub-domain fields, and callToGraph.
Files:
pkg/providers/dsx/client_test.gopkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
New or changed public behavior and new code paths should be covered by tests.
Files:
pkg/providers/dsx/client_test.gopkg/providers/dsx/instance_topology_test.go
⚙️ CodeRabbit configuration file
**/*_test.go: Flag missing negative, cancellation, concurrency, malformed-input,
boundary, and regression cases. Check that assertions would fail if
the behavior under test were broken.
Files:
pkg/providers/dsx/client_test.gopkg/providers/dsx/instance_topology_test.go
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Do not publicly disclose suspected security vulnerabilities; report them privately through NVIDIA PSIRT using the channels documented inSECURITY.md.
Every commit must include aSigned-off-by:trailer for DCO compliance.
Use Conventional Commits with an allowed type such asfeat,fix,docs,chore,refactor,test,build, orci.
Before pushing, runmake qualify; all CI checks, including Go build/test/lint, Codecov, and DCO, must be green before merge.
Changes to provider, engine, chart, API, configuration schema, labels, annotations, or user-facing behavior must include the corresponding documentation updates described in the documentation impact table.
Files:
pkg/providers/dsx/client_test.godocs/providers/dsx.mdpkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
{cmd,pkg,internal}/**/*.go
⚙️ CodeRabbit configuration file
{cmd,pkg,internal}/**/*.go: Focus on correctness, robustness, and failure paths:
- Check error handling and propagation, including whether callers
receive enough context to diagnose failures.- Verify context propagation, cancellation, timeouts, resource cleanup,
and goroutine lifecycle.- Look for races, deadlocks, unsafe shared state, leaks, and partial
updates that can leave state inconsistent.- Check nil, empty, malformed, duplicate, boundary, and partial inputs.
- Verify deterministic and idempotent behavior where operations may be
retried or repeated.- Preserve public contracts and the architectural boundaries documented
in AGENTS.md.- Flag behavior changes that lack meaningful regression coverage.
Files:
pkg/providers/dsx/client_test.gopkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
docs/providers/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Provider additions or changes require the corresponding provider documentation, including prerequisites, credentials, parameters, operation, and verification.
Files:
docs/providers/dsx.md
🔇 Additional comments (6)
docs/providers/dsx.md (1)
1-170: LGTM!pkg/providers/dsx/client_test.go (1)
1-62: LGTM!pkg/providers/dsx/instance_topology.go (2)
2-45: LGTM!
108-170: LGTM!pkg/providers/dsx/instance_topology_test.go (2)
63-242: LGTM!
465-491: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@pkg/providers/dsx/instance_topology_test.go`:
- Around line 40-54: Update mockClient.GetTopology to record a copy of the
nodeIDs argument, preserving each request’s IDs for assertions. Extend the
generateInstanceTopology tests with duplicate IDs across ComputeInstances
groups, then assert a request contains the sorted, unique IDs; ensure the
assertion would fail if deduplication or sorting were removed.
In `@pkg/providers/dsx/instance_topology.go`:
- Around line 153-167: Prevent duplicate fabric tiers in the tierIDs
construction around parentOf ancestry by tracking already emitted IDs and
skipping repeated spineID or coreID values, while preserving closest-first
ordering and non-empty filtering. Update pkg/providers/dsx/instance_topology.go
lines 153-167 and add an assertion in
pkg/providers/dsx/instance_topology_test.go lines 206-220 that cyclic input
produces unique FabricTiers IDs.
🪄 Autofix
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: ASSERTIVE
Plan: Enterprise
Run ID: 709bbaf3-42c0-4960-a8b5-281b41fd0aba
📒 Files selected for processing (3)
pkg/providers/dsx/client_test.gopkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (5)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Rungo fmt ./...; Go formatting is authoritative and code should not be hand-formatted.
Every new Go file must include the NVIDIA copyright header followed by the matching Apache 2.0 boilerplate.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.gopkg/providers/dsx/client_test.go
pkg/providers/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
pkg/providers/**/*.go: Providers must return a*topology.Graphand*httperr.Error; plainerroris not acceptable at the provider interface boundary.
Providers discover topology and return the canonicaltopology.Graph; they must not emit scheduler-specific output.
When usingClusterTopology, populate fabric tiers closest-first, preserve optional accelerator domain and sub-domain fields, and callToGraph.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.gopkg/providers/dsx/client_test.go
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Do not publicly disclose suspected security vulnerabilities; report them privately through NVIDIA PSIRT using the channels documented inSECURITY.md.
Every commit must include aSigned-off-by:trailer for DCO compliance.
Use Conventional Commits with an allowed type such asfeat,fix,docs,chore,refactor,test,build, orci.
Before pushing, runmake qualify; all CI checks, including Go build/test/lint, Codecov, and DCO, must be green before merge.
Changes to provider, engine, chart, API, configuration schema, labels, annotations, or user-facing behavior must include the corresponding documentation updates described in the documentation impact table.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.gopkg/providers/dsx/client_test.go
{cmd,pkg,internal}/**/*.go
⚙️ CodeRabbit configuration file
{cmd,pkg,internal}/**/*.go: Focus on correctness, robustness, and failure paths:
- Check error handling and propagation, including whether callers
receive enough context to diagnose failures.- Verify context propagation, cancellation, timeouts, resource cleanup,
and goroutine lifecycle.- Look for races, deadlocks, unsafe shared state, leaks, and partial
updates that can leave state inconsistent.- Check nil, empty, malformed, duplicate, boundary, and partial inputs.
- Verify deterministic and idempotent behavior where operations may be
retried or repeated.- Preserve public contracts and the architectural boundaries documented
in AGENTS.md.- Flag behavior changes that lack meaningful regression coverage.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.gopkg/providers/dsx/client_test.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
New or changed public behavior and new code paths should be covered by tests.
Files:
pkg/providers/dsx/instance_topology_test.gopkg/providers/dsx/client_test.go
⚙️ CodeRabbit configuration file
**/*_test.go: Flag missing negative, cancellation, concurrency, malformed-input,
boundary, and regression cases. Check that assertions would fail if
the behavior under test were broken.
Files:
pkg/providers/dsx/instance_topology_test.gopkg/providers/dsx/client_test.go
🔇 Additional comments (1)
pkg/providers/dsx/client_test.go (1)
1-244: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@pkg/providers/dsx/instance_topology_test.go`:
- Around line 408-425: Update the comment in
TestGenerateInstanceTopologyTotalTimeout to state that the stalled request must
return a 422 Unprocessable Entity, matching the http.StatusUnprocessableEntity
assertion and generateInstanceTopology behavior; leave the test logic unchanged.
In `@pkg/providers/dsx/instance_topology.go`:
- Around line 99-111: Update the apiErr handling around client.GetTopology in
the topology-generation flow to check the parent ctx cancellation before
treating genCtx.Err() as a total-generation deadline. Return a separate
cancellation error when ctx is canceled, while preserving the existing deadline
message for totalGenerationTimeout expiry and the BadGateway path for other API
errors.
🪄 Autofix
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: ASSERTIVE
Plan: Enterprise
Run ID: d61761d0-b63e-497f-b281-794047297cb5
📒 Files selected for processing (2)
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (5)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Rungo fmt ./...; Go formatting is authoritative and code should not be hand-formatted.
Every new Go file must include the NVIDIA copyright header followed by the matching Apache 2.0 boilerplate.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
pkg/providers/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
pkg/providers/**/*.go: Providers must return a*topology.Graphand*httperr.Error; plainerroris not acceptable at the provider interface boundary.
Providers discover topology and return the canonicaltopology.Graph; they must not emit scheduler-specific output.
When usingClusterTopology, populate fabric tiers closest-first, preserve optional accelerator domain and sub-domain fields, and callToGraph.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Do not publicly disclose suspected security vulnerabilities; report them privately through NVIDIA PSIRT using the channels documented inSECURITY.md.
Every commit must include aSigned-off-by:trailer for DCO compliance.
Use Conventional Commits with an allowed type such asfeat,fix,docs,chore,refactor,test,build, orci.
Before pushing, runmake qualify; all CI checks, including Go build/test/lint, Codecov, and DCO, must be green before merge.
Changes to provider, engine, chart, API, configuration schema, labels, annotations, or user-facing behavior must include the corresponding documentation updates described in the documentation impact table.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
{cmd,pkg,internal}/**/*.go
⚙️ CodeRabbit configuration file
{cmd,pkg,internal}/**/*.go: Focus on correctness, robustness, and failure paths:
- Check error handling and propagation, including whether callers
receive enough context to diagnose failures.- Verify context propagation, cancellation, timeouts, resource cleanup,
and goroutine lifecycle.- Look for races, deadlocks, unsafe shared state, leaks, and partial
updates that can leave state inconsistent.- Check nil, empty, malformed, duplicate, boundary, and partial inputs.
- Verify deterministic and idempotent behavior where operations may be
retried or repeated.- Preserve public contracts and the architectural boundaries documented
in AGENTS.md.- Flag behavior changes that lack meaningful regression coverage.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
New or changed public behavior and new code paths should be covered by tests.
Files:
pkg/providers/dsx/instance_topology_test.go
⚙️ CodeRabbit configuration file
**/*_test.go: Flag missing negative, cancellation, concurrency, malformed-input,
boundary, and regression cases. Check that assertions would fail if
the behavior under test were broken.
Files:
pkg/providers/dsx/instance_topology_test.go
🔇 Additional comments (5)
pkg/providers/dsx/instance_topology.go (3)
2-14: LGTM!Also applies to: 23-46, 54-70
112-126: LGTM!
128-187: LGTM!pkg/providers/dsx/instance_topology_test.go (2)
1-59: LGTM!Also applies to: 61-258
264-407: LGTM!Also applies to: 427-542
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@pkg/providers/dsx/instance_topology_test.go`:
- Around line 464-466: Update the test assertions around the page-limit error to
also verify that the mock client index is 3, confirming maxPaginationPages
prevents a fourth GetTopology request. Retain the existing error status and
message assertions.
- Around line 404-405: Update TestGenerateInstanceTopologyContextCancelled and
TestGenerateInstanceTopologyTotalTimeout to assert the expected error
classifications in addition to HTTP 422: “context cancelled” for caller
cancellation and “DSX topology generation deadline exceeded” for the total
timeout. Ensure these assertions distinguish the two error paths and fail if
generateInstanceTopology incorrectly routes either case through the other
classification.
In `@pkg/providers/dsx/instance_topology.go`:
- Around line 139-147: Make multi-key SwitchEntry processing deterministic in
the topology-building logic: collect and sort switch names before both
map-iteration passes, then use that ordered list when selecting parents and
appending instances. Preserve the existing warning for malformed entries, and
add a regression test covering a multi-key entry to verify deterministic,
idempotent ordering across repeated runs.
🪄 Autofix
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: ASSERTIVE
Plan: Enterprise
Run ID: 05ed62aa-c854-48a3-8cb7-652baea9e4b9
📒 Files selected for processing (2)
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (5)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Rungo fmt ./...; Go formatting is authoritative and code should not be hand-formatted.
Every new Go file must include the NVIDIA copyright header followed by the matching Apache 2.0 boilerplate.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
pkg/providers/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
pkg/providers/**/*.go: Providers must return a*topology.Graphand*httperr.Error; plainerroris not acceptable at the provider interface boundary.
Providers discover topology and return the canonicaltopology.Graph; they must not emit scheduler-specific output.
When usingClusterTopology, populate fabric tiers closest-first, preserve optional accelerator domain and sub-domain fields, and callToGraph.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Do not publicly disclose suspected security vulnerabilities; report them privately through NVIDIA PSIRT using the channels documented inSECURITY.md.
Every commit must include aSigned-off-by:trailer for DCO compliance.
Use Conventional Commits with an allowed type such asfeat,fix,docs,chore,refactor,test,build, orci.
Before pushing, runmake qualify; all CI checks, including Go build/test/lint, Codecov, and DCO, must be green before merge.
Changes to provider, engine, chart, API, configuration schema, labels, annotations, or user-facing behavior must include the corresponding documentation updates described in the documentation impact table.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
{cmd,pkg,internal}/**/*.go
⚙️ CodeRabbit configuration file
{cmd,pkg,internal}/**/*.go: Focus on correctness, robustness, and failure paths:
- Check error handling and propagation, including whether callers
receive enough context to diagnose failures.- Verify context propagation, cancellation, timeouts, resource cleanup,
and goroutine lifecycle.- Look for races, deadlocks, unsafe shared state, leaks, and partial
updates that can leave state inconsistent.- Check nil, empty, malformed, duplicate, boundary, and partial inputs.
- Verify deterministic and idempotent behavior where operations may be
retried or repeated.- Preserve public contracts and the architectural boundaries documented
in AGENTS.md.- Flag behavior changes that lack meaningful regression coverage.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
New or changed public behavior and new code paths should be covered by tests.
Files:
pkg/providers/dsx/instance_topology_test.go
⚙️ CodeRabbit configuration file
**/*_test.go: Flag missing negative, cancellation, concurrency, malformed-input,
boundary, and regression cases. Check that assertions would fail if
the behavior under test were broken.
Files:
pkg/providers/dsx/instance_topology_test.go
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 `@pkg/providers/dsx/instance_topology.go`:
- Around line 184-193: The buildClusterTopology logic in
pkg/providers/dsx/instance_topology.go lines 184-193 must exclude empty swName
values or otherwise construct tierIDs using only non-empty IDs, preventing empty
fabric tier IDs while preserving duplicate filtering. Add a malformed-input test
in pkg/providers/dsx/instance_topology_test.go lines 234-271 with a SwitchEntry
keyed by "" and assert that no emitted FabricTiers entry contains an empty ID.
🪄 Autofix
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: ASSERTIVE
Plan: Enterprise
Run ID: f3165864-adc1-44e1-8ec0-947cf73bfd45
📒 Files selected for processing (2)
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (5)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Rungo fmt ./...; Go formatting is authoritative and code should not be hand-formatted.
Every new Go file must include the NVIDIA copyright header followed by the matching Apache 2.0 boilerplate.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
pkg/providers/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
pkg/providers/**/*.go: Providers must return a*topology.Graphand*httperr.Error; plainerroris not acceptable at the provider interface boundary.
Providers discover topology and return the canonicaltopology.Graph; they must not emit scheduler-specific output.
When usingClusterTopology, populate fabric tiers closest-first, preserve optional accelerator domain and sub-domain fields, and callToGraph.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Do not publicly disclose suspected security vulnerabilities; report them privately through NVIDIA PSIRT using the channels documented inSECURITY.md.
Every commit must include aSigned-off-by:trailer for DCO compliance.
Use Conventional Commits with an allowed type such asfeat,fix,docs,chore,refactor,test,build, orci.
Before pushing, runmake qualify; all CI checks, including Go build/test/lint, Codecov, and DCO, must be green before merge.
Changes to provider, engine, chart, API, configuration schema, labels, annotations, or user-facing behavior must include the corresponding documentation updates described in the documentation impact table.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
{cmd,pkg,internal}/**/*.go
⚙️ CodeRabbit configuration file
{cmd,pkg,internal}/**/*.go: Focus on correctness, robustness, and failure paths:
- Check error handling and propagation, including whether callers
receive enough context to diagnose failures.- Verify context propagation, cancellation, timeouts, resource cleanup,
and goroutine lifecycle.- Look for races, deadlocks, unsafe shared state, leaks, and partial
updates that can leave state inconsistent.- Check nil, empty, malformed, duplicate, boundary, and partial inputs.
- Verify deterministic and idempotent behavior where operations may be
retried or repeated.- Preserve public contracts and the architectural boundaries documented
in AGENTS.md.- Flag behavior changes that lack meaningful regression coverage.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
New or changed public behavior and new code paths should be covered by tests.
Files:
pkg/providers/dsx/instance_topology_test.go
⚙️ CodeRabbit configuration file
**/*_test.go: Flag missing negative, cancellation, concurrency, malformed-input,
boundary, and regression cases. Check that assertions would fail if
the behavior under test were broken.
Files:
pkg/providers/dsx/instance_topology_test.go
🔇 Additional comments (9)
pkg/providers/dsx/instance_topology.go (4)
2-14: LGTM!Also applies to: 23-30
32-46: LGTM!
48-75: LGTM!
87-127: LGTM!pkg/providers/dsx/instance_topology_test.go (5)
1-65: LGTM!
71-233: LGTM!
289-358: LGTM!
360-452: LGTM!
454-570: LGTM!
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 `@pkg/providers/dsx/instance_topology.go`:
- Around line 58-63: The instance topology collection must reject or skip empty
instance IDs before updating want and nodeIDs, preventing Client.GetTopology
from receiving an empty node filter; update the instanceID loop in
pkg/providers/dsx/instance_topology.go:58-63 accordingly. Add coverage in
pkg/providers/dsx/instance_topology_test.go:535-543 for an empty-key
ComputeInstances.Instances case, asserting the chosen error behavior or that
GetTopology is not called.
🪄 Autofix
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: ASSERTIVE
Plan: Enterprise
Run ID: c97cc928-e782-48b0-8475-6c5a2604ee9b
📒 Files selected for processing (2)
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (5)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Rungo fmt ./...; Go formatting is authoritative and code should not be hand-formatted.
Every new Go file must include the NVIDIA copyright header followed by the matching Apache 2.0 boilerplate.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
pkg/providers/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
pkg/providers/**/*.go: Providers must return a*topology.Graphand*httperr.Error; plainerroris not acceptable at the provider interface boundary.
Providers discover topology and return the canonicaltopology.Graph; they must not emit scheduler-specific output.
When usingClusterTopology, populate fabric tiers closest-first, preserve optional accelerator domain and sub-domain fields, and callToGraph.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Do not publicly disclose suspected security vulnerabilities; report them privately through NVIDIA PSIRT using the channels documented inSECURITY.md.
Every commit must include aSigned-off-by:trailer for DCO compliance.
Use Conventional Commits with an allowed type such asfeat,fix,docs,chore,refactor,test,build, orci.
Before pushing, runmake qualify; all CI checks, including Go build/test/lint, Codecov, and DCO, must be green before merge.
Changes to provider, engine, chart, API, configuration schema, labels, annotations, or user-facing behavior must include the corresponding documentation updates described in the documentation impact table.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
{cmd,pkg,internal}/**/*.go
⚙️ CodeRabbit configuration file
{cmd,pkg,internal}/**/*.go: Focus on correctness, robustness, and failure paths:
- Check error handling and propagation, including whether callers
receive enough context to diagnose failures.- Verify context propagation, cancellation, timeouts, resource cleanup,
and goroutine lifecycle.- Look for races, deadlocks, unsafe shared state, leaks, and partial
updates that can leave state inconsistent.- Check nil, empty, malformed, duplicate, boundary, and partial inputs.
- Verify deterministic and idempotent behavior where operations may be
retried or repeated.- Preserve public contracts and the architectural boundaries documented
in AGENTS.md.- Flag behavior changes that lack meaningful regression coverage.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
New or changed public behavior and new code paths should be covered by tests.
Files:
pkg/providers/dsx/instance_topology_test.go
⚙️ CodeRabbit configuration file
**/*_test.go: Flag missing negative, cancellation, concurrency, malformed-input,
boundary, and regression cases. Check that assertions would fail if
the behavior under test were broken.
Files:
pkg/providers/dsx/instance_topology_test.go
🔇 Additional comments (4)
pkg/providers/dsx/instance_topology.go (2)
2-46: LGTM!
72-142: LGTM!pkg/providers/dsx/instance_topology_test.go (2)
31-533: LGTM!
545-608: LGTM!
9df43dc to
51763db
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@pkg/providers/dsx/instance_topology_test.go`:
- Around line 210-232: Strengthen the checkInst assertion in the “multi-key
SwitchEntry is processed deterministically” test to verify that topo.Instances
is emitted in the exact ID order ["n1", "n2"], while retaining the existing
fabric-tier checks. Use an order-sensitive assertion so the test fails if
sortedEntryKeys is removed and map iteration changes the instance order.
In `@pkg/providers/dsx/instance_topology.go`:
- Around line 117-120: Classify non-cancellation DSX client failures as gateway
errors by changing the error status returned in
pkg/providers/dsx/instance_topology.go lines 117-120 from
http.StatusUnprocessableEntity to http.StatusBadGateway while preserving the
existing context. Update the expectation in
pkg/providers/dsx/instance_topology_test.go lines 375-392 for “backend
unavailable” to use http.StatusBadGateway.
🪄 Autofix
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: ASSERTIVE
Plan: Enterprise
Run ID: 607c0a28-fbc5-4471-908f-34053876391f
📒 Files selected for processing (4)
pkg/providers/dsx/client.gopkg/providers/dsx/client_test.gopkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (5)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Rungo fmt ./...; Go formatting is authoritative and code should not be hand-formatted.
Every new Go file must include the NVIDIA copyright header followed by the matching Apache 2.0 boilerplate.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/client.gopkg/providers/dsx/instance_topology_test.gopkg/providers/dsx/client_test.go
pkg/providers/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
pkg/providers/**/*.go: Providers must return a*topology.Graphand*httperr.Error; plainerroris not acceptable at the provider interface boundary.
Providers discover topology and return the canonicaltopology.Graph; they must not emit scheduler-specific output.
When usingClusterTopology, populate fabric tiers closest-first, preserve optional accelerator domain and sub-domain fields, and callToGraph.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/client.gopkg/providers/dsx/instance_topology_test.gopkg/providers/dsx/client_test.go
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Do not publicly disclose suspected security vulnerabilities; report them privately through NVIDIA PSIRT using the channels documented inSECURITY.md.
Every commit must include aSigned-off-by:trailer for DCO compliance.
Use Conventional Commits with an allowed type such asfeat,fix,docs,chore,refactor,test,build, orci.
Before pushing, runmake qualify; all CI checks, including Go build/test/lint, Codecov, and DCO, must be green before merge.
Changes to provider, engine, chart, API, configuration schema, labels, annotations, or user-facing behavior must include the corresponding documentation updates described in the documentation impact table.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/client.gopkg/providers/dsx/instance_topology_test.gopkg/providers/dsx/client_test.go
{cmd,pkg,internal}/**/*.go
⚙️ CodeRabbit configuration file
{cmd,pkg,internal}/**/*.go: Focus on correctness, robustness, and failure paths:
- Check error handling and propagation, including whether callers
receive enough context to diagnose failures.- Verify context propagation, cancellation, timeouts, resource cleanup,
and goroutine lifecycle.- Look for races, deadlocks, unsafe shared state, leaks, and partial
updates that can leave state inconsistent.- Check nil, empty, malformed, duplicate, boundary, and partial inputs.
- Verify deterministic and idempotent behavior where operations may be
retried or repeated.- Preserve public contracts and the architectural boundaries documented
in AGENTS.md.- Flag behavior changes that lack meaningful regression coverage.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/client.gopkg/providers/dsx/instance_topology_test.gopkg/providers/dsx/client_test.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
New or changed public behavior and new code paths should be covered by tests.
Files:
pkg/providers/dsx/instance_topology_test.gopkg/providers/dsx/client_test.go
⚙️ CodeRabbit configuration file
**/*_test.go: Flag missing negative, cancellation, concurrency, malformed-input,
boundary, and regression cases. Check that assertions would fail if
the behavior under test were broken.
Files:
pkg/providers/dsx/instance_topology_test.gopkg/providers/dsx/client_test.go
🔇 Additional comments (2)
pkg/providers/dsx/client.go (1)
1-106: LGTM!pkg/providers/dsx/client_test.go (1)
32-256: LGTM!
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 `@pkg/providers/dsx/instance_topology_test.go`:
- Around line 242-246: Update the checkInst assertion in the
buildClusterTopology test case for the empty SwitchEntry key to assert the exact
expected length of topo.Instances[0].FabricTiers before validating tier IDs,
ensuring the test fails when tiers are unexpectedly omitted or emitted.
🪄 Autofix
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: ASSERTIVE
Plan: Enterprise
Run ID: a5f73d7b-96b8-4e42-8dd4-19c7dfdcccf4
📒 Files selected for processing (2)
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Rungo fmt ./...; Go formatting is authoritative and code should not be hand-formatted.
Every new Go file must include the NVIDIA copyright header followed by the matching Apache 2.0 boilerplate.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
pkg/providers/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
pkg/providers/**/*.go: Providers must return a*topology.Graphand*httperr.Error; plainerroris not acceptable at the provider interface boundary.
Providers discover topology and return the canonicaltopology.Graph; they must not emit scheduler-specific output.
When usingClusterTopology, populate fabric tiers closest-first, preserve optional accelerator domain and sub-domain fields, and callToGraph.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Do not publicly disclose suspected security vulnerabilities; report them privately through NVIDIA PSIRT using the channels documented inSECURITY.md.
Every commit must include aSigned-off-by:trailer for DCO compliance.
Use Conventional Commits with an allowed type such asfeat,fix,docs,chore,refactor,test,build, orci.
Before pushing, runmake qualify; all CI checks, including Go build/test/lint, Codecov, and DCO, must be green before merge.
Changes to provider, engine, chart, API, configuration schema, labels, annotations, or user-facing behavior must include the corresponding documentation updates described in the documentation impact table.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
{cmd,pkg,internal}/**/*.go
⚙️ CodeRabbit configuration file
{cmd,pkg,internal}/**/*.go: Focus on correctness, robustness, and failure paths:
- Check error handling and propagation, including whether callers
receive enough context to diagnose failures.- Verify context propagation, cancellation, timeouts, resource cleanup,
and goroutine lifecycle.- Look for races, deadlocks, unsafe shared state, leaks, and partial
updates that can leave state inconsistent.- Check nil, empty, malformed, duplicate, boundary, and partial inputs.
- Verify deterministic and idempotent behavior where operations may be
retried or repeated.- Preserve public contracts and the architectural boundaries documented
in AGENTS.md.- Flag behavior changes that lack meaningful regression coverage.
Files:
pkg/providers/dsx/instance_topology.gopkg/providers/dsx/instance_topology_test.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
New or changed public behavior and new code paths should be covered by tests.
Files:
pkg/providers/dsx/instance_topology_test.go
⚙️ CodeRabbit configuration file
**/*_test.go: Flag missing negative, cancellation, concurrency, malformed-input,
boundary, and regression cases. Check that assertions would fail if
the behavior under test were broken.
Files:
pkg/providers/dsx/instance_topology_test.go
🔇 Additional comments (8)
pkg/providers/dsx/instance_topology.go (5)
2-14: LGTM!Also applies to: 23-47
49-80: LGTM!
82-136: LGTM!
138-164: LGTM!
166-213: LGTM!pkg/providers/dsx/instance_topology_test.go (3)
1-65: LGTM!
303-509: LGTM!
511-595: LGTM!
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 `@pkg/providers/dsx/instance_topology_test.go`:
- Around line 395-429: Strengthen both cycle-detection tests for
generateInstanceTopology: after the error assertions in
TestGenerateInstanceTopologyDirectCyclePageToken, assert mc.idx equals 2, and
after the assertions in
TestGenerateInstanceTopologyNonConsecutiveCyclePageToken, assert mc.idx equals
3. This verifies pagination stops immediately when a repeated token is received
and prevents an extra GetTopology request.
🪄 Autofix
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: ASSERTIVE
Plan: Enterprise
Run ID: a11eb80c-72de-4de7-9e9e-ad65f54d605d
📒 Files selected for processing (1)
pkg/providers/dsx/instance_topology_test.go
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (5)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Rungo fmt ./...; Go formatting is authoritative and code should not be hand-formatted.
Every new Go file must include the NVIDIA copyright header followed by the matching Apache 2.0 boilerplate.
Files:
pkg/providers/dsx/instance_topology_test.go
pkg/providers/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
pkg/providers/**/*.go: Providers must return a*topology.Graphand*httperr.Error; plainerroris not acceptable at the provider interface boundary.
Providers discover topology and return the canonicaltopology.Graph; they must not emit scheduler-specific output.
When usingClusterTopology, populate fabric tiers closest-first, preserve optional accelerator domain and sub-domain fields, and callToGraph.
Files:
pkg/providers/dsx/instance_topology_test.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
New or changed public behavior and new code paths should be covered by tests.
Files:
pkg/providers/dsx/instance_topology_test.go
⚙️ CodeRabbit configuration file
**/*_test.go: Flag missing negative, cancellation, concurrency, malformed-input,
boundary, and regression cases. Check that assertions would fail if
the behavior under test were broken.
Files:
pkg/providers/dsx/instance_topology_test.go
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Do not publicly disclose suspected security vulnerabilities; report them privately through NVIDIA PSIRT using the channels documented inSECURITY.md.
Every commit must include aSigned-off-by:trailer for DCO compliance.
Use Conventional Commits with an allowed type such asfeat,fix,docs,chore,refactor,test,build, orci.
Before pushing, runmake qualify; all CI checks, including Go build/test/lint, Codecov, and DCO, must be green before merge.
Changes to provider, engine, chart, API, configuration schema, labels, annotations, or user-facing behavior must include the corresponding documentation updates described in the documentation impact table.
Files:
pkg/providers/dsx/instance_topology_test.go
{cmd,pkg,internal}/**/*.go
⚙️ CodeRabbit configuration file
{cmd,pkg,internal}/**/*.go: Focus on correctness, robustness, and failure paths:
- Check error handling and propagation, including whether callers
receive enough context to diagnose failures.- Verify context propagation, cancellation, timeouts, resource cleanup,
and goroutine lifecycle.- Look for races, deadlocks, unsafe shared state, leaks, and partial
updates that can leave state inconsistent.- Check nil, empty, malformed, duplicate, boundary, and partial inputs.
- Verify deterministic and idempotent behavior where operations may be
retried or repeated.- Preserve public contracts and the architectural boundaries documented
in AGENTS.md.- Flag behavior changes that lack meaningful regression coverage.
Files:
pkg/providers/dsx/instance_topology_test.go
🔇 Additional comments (1)
pkg/providers/dsx/instance_topology_test.go (1)
1-394: LGTM!Also applies to: 431-597
Signed-off-by: Ravi Shankar <ravish@nvidia.com>
2 participants
Description
Added dsx provider implementation