Skip to content

Commit f901d70

Browse files
committed
admin: refresh stale ErrTablesNotLeader sentinel comment (Claude review)
Two nit follow-ups from Claude review #4 on 87043f9: 1) The ErrTablesNotLeader sentinel's own declaration comment still said "the future AdminForward RPC catches this as the trigger to forward". The previous review pass updated only the writeTablesError site comment. Refreshed the sentinel comment too: when a LeaderForwarder is configured, tryForwardCreate / tryForwardDelete catch this before writeTablesError; without a forwarder, the arm maps to 503 + Retry-After:1. 2) newFollowerHandler still constructed notLeaderSource with an empty tables map. notLeaderSource overrides AdminCreateTable / AdminDeleteTable to unconditionally return ErrTablesNotLeader, so the map is never consulted. The two individual tests cleaned up in the previous commit already use bare &notLeaderSource{}; the helper now matches.
1 parent 87043f9 commit f901d70

2 files changed

Lines changed: 8 additions & 3 deletions

File tree

internal/admin/dynamo_handler.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,10 @@ var (
119119
// role required for the operation. Maps to 403.
120120
ErrTablesForbidden = errors.New("admin tables: principal lacks required role")
121121
// ErrTablesNotLeader is returned when the local node is not the
122-
// Raft leader. Maps to 503 + Retry-After: 1 today; the future
123-
// AdminForward RPC catches this as the trigger to forward.
122+
// Raft leader. When a LeaderForwarder is configured,
123+
// tryForwardCreate / tryForwardDelete catch this before it reaches
124+
// writeTablesError and forward the request to the leader
125+
// transparently. Without a forwarder, maps to 503 + Retry-After: 1.
124126
ErrTablesNotLeader = errors.New("admin tables: local node is not the raft leader")
125127
// ErrTablesNotFound is returned when DELETE / DESCRIBE / a
126128
// follow-up read targets a table that does not exist. Maps to

internal/admin/forward_integration_test.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,10 @@ func (s *notLeaderSource) AdminDeleteTable(_ context.Context, _ AuthPrincipal, _
6363

6464
func newFollowerHandler(t *testing.T, fwd LeaderForwarder) *DynamoHandler {
6565
t.Helper()
66-
src := &notLeaderSource{stubTablesSource: stubTablesSource{tables: map[string]*DynamoTableSummary{}}}
66+
// notLeaderSource overrides the write methods to unconditionally
67+
// return ErrTablesNotLeader, so the embedded stubTablesSource
68+
// never reads its tables map.
69+
src := &notLeaderSource{}
6770
h := NewDynamoHandler(src)
6871
if fwd != nil {
6972
h = h.WithLeaderForwarder(fwd)

0 commit comments

Comments
 (0)