Skip to content

fix(provider/lambdai): align client with the Lambda topology API#374

Open
alaski-lambda wants to merge 1 commit into
NVIDIA:mainfrom
alaski-lambda:fix/lambdai-topology-api
Open

fix(provider/lambdai): align client with the Lambda topology API#374
alaski-lambda wants to merge 1 commit into
NVIDIA:mainfrom
alaski-lambda:fix/lambdai-topology-api

Conversation

@alaski-lambda

@alaski-lambda alaski-lambda commented Jun 30, 2026

Copy link
Copy Markdown

Description

Match the deployed API: query GET /api/v1/topology/instance with the required region parameter, parse the {data, page_token} envelope and paginate via page_token, and model networkPath as {id} objects mapped to leaf/spine/core. Update the simulator and add an httptest covering the request contract, envelope, pagination, and mapping.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.
  • All commits are signed off per DCO (git commit -s).

Match the deployed API: query GET /api/v1/topology/instance with the
required region parameter, parse the {data, page_token} envelope and
paginate via page_token, and model networkPath as {id} objects mapped to
leaf/spine/core. Update the simulator and add an httptest covering the
request contract, envelope, pagination, and mapping.

Signed-off-by: Andrew Laski <alaski@lambdal.com>
@copy-pr-bot

copy-pr-bot Bot commented Jun 30, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@greptile-apps

greptile-apps Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR aligns the Lambda AI provider client with the deployed topology API by correcting the endpoint path (/api/v1/topology/instance), adding the required region query parameter, parsing the {data, page_token} response envelope, and changing networkPath elements from bare strings to {id} objects (NetworkHop). A new httptest-based test validates the full request contract, pagination, and leaf/spine/core mapping end-to-end.

  • provider.go: Introduces apiPath, apiResponse, and NetworkHop types; InstanceListRequest gains Region; InstanceList now sends region in the query and correctly drives pagination via page_token from the envelope.
  • instance_topology.go / provider_sim.go: Updated call sites to supply Region and access NetworkPath[i].ID instead of the old bare string.
  • provider_test.go: Adds TestGenerateTopologyConfig with a mock HTTP server covering the two-page pagination flow and node→switch hierarchy mapping.

Confidence Score: 4/5

The change is straightforward API alignment with a well-scoped impact; the pagination loop, region validation, and envelope parsing are all correct.

The core logic is correct: region is validated before use, JSON null for page_token correctly unmarshals to empty string and terminates the loop, and the NetworkHop.ID mapping matches the documented API shape. The new httptest test provides good contract coverage. The only finding is a test-safety issue — require.Equal inside the httptest handler goroutine can panic rather than cleanly fail the test — which does not affect production behavior but could make test failures harder to diagnose.

pkg/providers/lambdai/provider_test.go — the handler goroutine assertion pattern.

Important Files Changed

Filename Overview
pkg/providers/lambdai/provider.go Aligned with the Lambda topology API: new apiPath constant, apiResponse envelope struct, NetworkHop struct replacing []string, Region added to InstanceListRequest, and pagination correctly driven by page_token from the envelope.
pkg/providers/lambdai/instance_topology.go Passes Region into InstanceListRequest and accesses NetworkPath entries via .ID after the []string → []NetworkHop change; pagination loop and region guard unchanged and correct.
pkg/providers/lambdai/provider_sim.go Simulator updated to construct []NetworkHop from node.NetLayers, matching the new InstanceTopology shape; rest of simClient unchanged.
pkg/providers/lambdai/provider_test.go New TestGenerateTopologyConfig uses httptest to cover path, region, workspace_id, auth, envelope parsing, pagination, and leaf/spine/core mapping; one minor test-safety issue with require.Equal inside the handler goroutine.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant C as GenerateTopologyConfig
    participant L as InstanceList (lambdaiClient)
    participant API as Lambda /api/v1/topology/instance

    C->>L: "InstanceListRequest{Region, PageSize}"
    L->>API: "GET ?workspace_id=&region=&page_size="
    API-->>L: "{"data":[...], "page_token":"tok2"}"
    L-->>C: "InstanceListResponse{Items, NextPageToken="tok2"}"

    C->>L: "InstanceListRequest{Region, PageSize, PageToken="tok2"}"
    L->>API: "GET ?workspace_id=&region=&page_size=&page_token=tok2"
    API-->>L: "{"data":[...], "page_token":null}"
    L-->>C: "InstanceListResponse{Items, NextPageToken=""}"

    Note over C: NextPageToken == "" → stop
    C->>C: Build ClusterTopology from NetworkHop.ID (leaf/spine/core)
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant C as GenerateTopologyConfig
    participant L as InstanceList (lambdaiClient)
    participant API as Lambda /api/v1/topology/instance

    C->>L: "InstanceListRequest{Region, PageSize}"
    L->>API: "GET ?workspace_id=&region=&page_size="
    API-->>L: "{"data":[...], "page_token":"tok2"}"
    L-->>C: "InstanceListResponse{Items, NextPageToken="tok2"}"

    C->>L: "InstanceListRequest{Region, PageSize, PageToken="tok2"}"
    L->>API: "GET ?workspace_id=&region=&page_size=&page_token=tok2"
    API-->>L: "{"data":[...], "page_token":null}"
    L-->>C: "InstanceListResponse{Items, NextPageToken=""}"

    Note over C: NextPageToken == "" → stop
    C->>C: Build ClusterTopology from NetworkHop.ID (leaf/spine/core)
Loading

Reviews (1): Last reviewed commit: "fix(provider/lambdai): align client with..." | Re-trigger Greptile


var paths, regions, workspaces, auths []string
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
require.Equal(t, http.MethodGet, r.Method)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 require inside handler goroutine may panic instead of failing cleanly

require.Equal calls t.FailNow(), which per Go's testing docs must only be called from the goroutine running the test function. Calling it from a goroutine managed by httptest.Server can cause a panic (via runtime.Goexit) rather than a clean test failure. Using assert.Equal (which calls t.Fail() and returns normally) is the safe pattern for assertions inside HTTP handlers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant