Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions internal/api/client.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -346,26 +346,32 @@ func (c *Client) ListClients(ctx context.Context) ([]ProvisionedClient, error) {
return nil, fmt.Errorf("decoding client list: %w", err)
}
all = append(all, body.Results...)
path = nextPath(body.Next)
next, perr := nextPath(body.Next)
if perr != nil {
return nil, perr
}
path = next
}
return all, nil
}

// nextPath reduces a DRF `next` link (an absolute URL) to the path+query this
// client appends to BaseURL. Returns "" for an empty/unparseable link, which
// ends the pagination loop.
func nextPath(next string) string {
// client appends to BaseURL. An empty link returns ("", nil) — the normal
// end of pages. A non-empty link that won't parse is an error, NOT a silent
// "", so the loop never quietly stops mid-list and returns only the pages
// seen so far (list / `use` / collision checks must see every client).
func nextPath(next string) (string, error) {
if next == "" {
return ""
return "", nil
}
u, err := url.Parse(next)
if err != nil {
return ""
return "", fmt.Errorf("client list: unparseable pagination link %q: %w", next, err)
}
if u.RawQuery != "" {
return u.Path + "?" + u.RawQuery
return u.Path + "?" + u.RawQuery, nil
}
return u.Path
return u.Path, nil
}

// ListClientAdmins returns who in the account can provision (the ask-an-admin
Expand Down
40 changes: 40 additions & 0 deletions internal/api/client_test.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -194,3 +194,43 @@ func TestCreateClientConflict(t *testing.T) {
t.Errorf("want APIError 409, got %v", err)
}
}

// TestListClients_FollowsPagination guards that DRF pagination is still
// followed end-to-end after the nextPath refactor (page 1 → page 2 → done).
func TestListClients_FollowsPagination(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Query().Get("page") == "2" {
_, _ = w.Write([]byte(`{"next":"","results":[{"id":2,"first_name":"b","namespace":"b"}]}`))
return
}
// DRF emits an absolute `next`; nextPath keeps only path+query.
_, _ = w.Write([]byte(`{"next":"http://x/edge-device/?page=2","results":[{"id":1,"first_name":"a","namespace":"a"}]}`))
}))
defer srv.Close()
c := New("prod")
c.BaseURL = srv.URL
got, err := c.ListClients(context.Background())
if err != nil {
t.Fatal(err)
}
if len(got) != 2 || got[0].ID != 1 || got[1].ID != 2 {
t.Fatalf("want 2 clients [1,2], got %+v", got)
}
}

// TestListClients_UnparseableNextLink_IsError pins the Bugbot fix (v0.4.0 RC):
// a non-empty `next` the server sends that url.Parse rejects must be a hard
// error, never a silent truncation to the pages seen so far — otherwise list /
// `use` / namespace-collision checks would miss clients without any error.
func TestListClients_UnparseableNextLink_IsError(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
// `next` carries a control byte (\u007f) → url.Parse fails.
_, _ = w.Write([]byte(`{"next":"http://x/\u007f","results":[{"id":1,"first_name":"a","namespace":"a"}]}`))
}))
defer srv.Close()
c := New("prod")
c.BaseURL = srv.URL
if _, err := c.ListClients(context.Background()); err == nil {
t.Fatal("expected an error on an unparseable next link, got nil (silent truncation)")
}
}
11 changes: 7 additions & 4 deletions internal/cli/dataset.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -423,10 +423,13 @@ contributors train against it without ever seeing the raw files.`))
// "category is required" error downstream.
case push.IsCLISupported(a.Spec.Category):
// supported
case push.IsImage(a.Spec.Category):
// A known image category dataset push doesn't implement yet
// (semantic_segmentation / instance_segmentation). The per-category
// reason + the supported list both come from the registry.
case push.IsKnown(a.Spec.Category):
// A recognized category dataset push doesn't implement yet — image
// (semantic_segmentation / instance_segmentation) or text
// (causal_language_modeling). Routed here (not the default branch) so the
// user gets the registry's per-category pending-support reason, not a
// misleading "unrecognized category". Supported categories were already
// caught above, so IsKnown here means known-but-unsupported.
spec, _ := push.Lookup(a.Spec.Category)
return &exitError{code: 2, err: fmt.Errorf(
"category %q isn't supported by the CLI yet (%s). Supported categories: %s.",
Expand Down
31 changes: 31 additions & 0 deletions internal/cli/dataset_test.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -93,6 +93,37 @@ func TestDatasetPush_UnsupportedCategory_ExitsTwo(t *testing.T) {
}
}

// TestDatasetPush_KnownUnsupportedCategory_PendingNote pins the Bugbot fix
// (v0.4.0 RC): a registry-known but CLI-unsupported NON-image category
// (causal_language_modeling) must get the registry's pending-support note, not
// the misleading "isn't a recognized task category" message. execDatasetPush
// discards the error and SilenceErrors swallows it, so run the command here and
// inspect the returned error directly.
func TestDatasetPush_KnownUnsupportedCategory_PendingNote(t *testing.T) {
root := imgcLayout(t)
rootCmd := NewRootCmd(BuildInfo{Version: "test"})
rootCmd.SetOut(&bytes.Buffer{})
rootCmd.SetErr(&bytes.Buffer{})
rootCmd.SetArgs([]string{"dataset", "push",
"--kubeconfig=/tmp/tracebloc-cli-test-nonexistent-" + t.Name(),
root, "--table=t1", "--category=causal_language_modeling",
"--intent=train", "--label-column=label"})
err := rootCmd.Execute()
if err == nil {
t.Fatal("expected an error for a known-but-unsupported category")
}
if got := ExitCodeFromError(err); got != 2 {
t.Fatalf("exit code = %d, want 2", got)
}
msg := err.Error()
if strings.Contains(msg, "isn't a recognized task category") {
t.Errorf("known category misrouted to the unrecognized-category branch:\n%s", msg)
}
if !strings.Contains(msg, "isn't supported by the CLI yet") {
t.Errorf("want the registry pending-support note, got:\n%s", msg)
}
}

// TestDatasetPush_TraversalTableName_ExitsTwo is the security
// regression pin at the CLI layer. --table=../../etc must be
// rejected with exit 2 BEFORE any spec synthesis or cluster work —
Expand Down
Loading