Skip to content

atepg: name the row in list decode errors - #2000

Merged
Luiz Oliveira (laoj2) merged 3 commits into
agent-substrate:mainfrom
ericcurtin:atepg-name-bad-row
Sep 30, 2026
Merged

Luiz Oliveira (laoj2) merged 3 commits into
agent-substrate:mainfrom
ericcurtin:atepg-name-bad-row

Conversation

@ericcurtin

Copy link
Copy Markdown
Contributor

Part of #1401.

A listing fails on one undecodable row. The error now names it.

  • Tests pass
  • Appropriate changes to documentation are included in the PR

One bad row fails the whole listing, so say which.
@@ -297,7 +297,7 @@ func (p *Persistence) ListWorkerAssignments(ctx context.Context, workerName stri
}
assignment := &ateapipb.ActorAssignment{}
if err := proto.Unmarshal(protoBytes, assignment); err != nil {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

what about ListWorkers?

if err := unmarshalStored(protoBytes, w); err != nil {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done, ListWorkers now names the row too (and is in the test).

list func() error
want string
}{
{"atespaces", func() error { _, err := s.ListAtespaces(ctx, opts); return err }, "atespace team-a"},

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: the table cases in this repo use positional fields on one line each, not named fields per case

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The cases are already positional, one line each. Did you mean something else here?

@@ -0,0 +1,83 @@
// Copyright 2026 Google LLC

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can we rename this file to atepg_test.go? so we can add tests that don't fit the _test.go pattern

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done, renamed to atepg_test.go.

@laoj2
Luiz Oliveira (laoj2) added this pull request to the merge queue Sep 30, 2026
Merged via the queue into agent-substrate:main with commit f0e3ef6 Sep 30, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/api-machinery kind/cleanup Small fixes that are not bugs, for example a typo in a code comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants