Skip to content

[4/N] [List API] feat(gateway): expose List API from request summaries#304

Open
albertywu wants to merge 1 commit into
wua/list-api-summary-storefrom
wua/list-api-gateway-rpc
Open

[4/N] [List API] feat(gateway): expose List API from request summaries#304
albertywu wants to merge 1 commit into
wua/list-api-summary-storefrom
wua/list-api-gateway-rpc

Conversation

@albertywu

@albertywu albertywu commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds the public gateway List RPC and controller for queue-scoped request summaries with queue validation, status filtering, deterministic pagination, and admission-time sorting. Gateway write paths now persist log entries through the summary-aware helper so the read model stays updated.

Test Plan

make fmt && make build && make test && make check-mocks && make e2e-test

Issues

Stack

  1. [1/N] [List API] docs(list): define request context read model #310
  2. [2/N] [List API] feat(context): add immutable request admission store #302
  3. [3/N] [List API] feat(summary): project request logs from admission context #303
  4. @ [4/N] [List API] feat(gateway): expose List API from request summaries #304
  5. [5/N] [List API] test(e2e): cover gateway List API #305
  6. [6/N] [List API] docs(service): document gateway List API read model #306

@albertywu albertywu force-pushed the wua/list-api-summary-store branch from 60d9640 to 552ce82 Compare July 7, 2026 00:22
@albertywu albertywu force-pushed the wua/list-api-gateway-rpc branch 2 times, most recently from 61b04fe to d0e9c2a Compare July 7, 2026 00:40
@albertywu albertywu force-pushed the wua/list-api-summary-store branch from 552ce82 to 419a487 Compare July 7, 2026 00:40
@albertywu albertywu force-pushed the wua/list-api-gateway-rpc branch from d0e9c2a to 96cfc64 Compare July 7, 2026 00:46
@albertywu albertywu force-pushed the wua/list-api-summary-store branch from 419a487 to 4980cb0 Compare July 7, 2026 00:46
@albertywu albertywu marked this pull request as ready for review July 7, 2026 01:42
@albertywu albertywu requested review from a team, behinddwalls and sbalabanov as code owners July 7, 2026 01:42
@albertywu albertywu force-pushed the wua/list-api-summary-store branch from 4980cb0 to 788ef8d Compare July 7, 2026 01:49
@albertywu albertywu force-pushed the wua/list-api-gateway-rpc branch from 96cfc64 to d308ee7 Compare July 7, 2026 01:49
@albertywu albertywu changed the title feat(gateway): expose request List RPC [3/N] [List API] feat(gateway): expose request List RPC Jul 7, 2026
@behinddwalls

Copy link
Copy Markdown
Collaborator

Few thoughts on this.

1. Group filters vs. pagination

ListRequest is a bit hard to read at a glance — filters, ordering, and pagination sit flat in one message, so it's unclear what's required, what's optional, and what's meant to combine. Grouping by role would make it more self-documenting.

On whether filters should be a generic Filter{key, value} list for extensibility: I'd lean against it. A proto message with named fields already is a typed key/value model — the keys are fields the schema knows. A Filter{key, value} list gives that up: keys move into runtime data, values collapse to strings, and we'd reinvent operators and validation. So I'd keep filters as named typed fields, grouped into a sub-message. (If we ever need open-ended user-composed predicates, the usual path is a single string filter grammar à la AIP-160 — but not yet.)

2. A possible restructure

A starting point:

message ListRequest {
  // Required: the queue to scan (storage partition key).
  string queue = 1;
  // Optional. Absent = whole queue.
  // Present fields AND together; a repeated field ORs within itself.
  ListFilter filter = 2;
  // Optional ordering; unspecified = admission order (FIFO).
  ListSort sort = 3;
  // Optional pagination, independent of filtering.
  Page page = 4;
}

message ListFilter {
  TimeWindow window = 1;          // either bound 0 = unbounded
  repeated string statuses = 2;   // OR-set; empty = all
}

// Half-open [start, end) in Unix epoch ms. 0 = unbounded.
message TimeWindow {
  int64 start_time_ms = 1;
  int64 end_time_ms = 2;
}

message Page {
  int32 size = 1;   // 0 = server default; server may cap
  string token = 2; // opaque continuation (see below)
}

enum ListSort {
  LIST_SORT_UNSPECIFIED  = 0;  // = admission ascending
  LIST_SORT_ADMITTED_ASC = 1;
  LIST_SORT_ADMITTED_DESC = 2;
}

Small things folded in:

  • proto3 optional on the time bounds would distinguish "no lower bound" from start_time_ms = 0.
  • A oneof could encode any mutually-exclusive filters in the schema rather than in prose.
  • I'd suggest dropping the terminal filter: it overlaps statuses, and terminal=true + statuses=[building] is a contradictory request the server then has to define. Terminality is derivable from the status set. Worth keeping the derived terminal bool on the response RequestSummary though — as output it saves clients from hardcoding which statuses are terminal.

3. In-flight requests in the time window

One edge case worth a look: the contract says "lifecycles overlap a time window," but the storage filter (parent PR) is started_at_ms < end AND completed_at_ms > start, and in-flight requests have completed_at_ms = 0. So 0 > start is false whenever start_time_ms > 0 — still-running requests get filtered out, which seems opposite to what a "queue right now" view wants. Treating 0 as open-ended (completed_at_ms = 0 OR completed_at_ms > ?) would fix it, and might argue for allowing an unbounded window. Let me know if I've misread the query.

4. Pagination token — a note and an open question

Embedding the query in the token looks safe: values go through bound ? parameters and sort/order comes from a whitelist switch, so no injection path, and the token carries no entitlements — a tampered one only moves the caller's own cursor within an already-authorized query. Might be worth stating as an invariant: the token can hold position + a query echo, never anything the server treats as permission.

Optional for later: if we ever want forged tokens to be impossible rather than just harmless, we could HMAC-sign the token with a server-side secret. The signature also guarantees integrity, so the token could shrink to position + a query hash instead of echoing the whole query. Cost is key management, so probably a follow-up unless the token needs to carry something sensitive. A length cap before decoding is a cheap safeguard regardless.

Open one, no strong opinion — token or cursor?

  • token matches the AIP convention and the internal listPageToken, and signals "opaque, tied to this query, don't introspect" — which fits, since it's position + frozen query and we reject on mismatch. Keeps it distinct from the storage RequestSummaryCursor (the actual position).
  • cursor is more familiar to consumers (GraphQL/Relay), but implies a plain reusable pointer, which is at odds with the mismatch rejection. If we preferred it, we'd probably make it a true position-only cursor.

Curious what you think — feels like a judgment call, and it interacts with the HMAC question.

5. Tiny things

  • ListSort may trip buf/AIP lint (zero-value naming + unprefixed values) — the LIST_SORT_UNSPECIFIED + prefixed form above should fix it.
  • statuses as free strings seems reasonable (mirrors StatusResponse.status), just worth documenting intent for unknown values: today a hard error, but we could treat them as ignored so an older client filtering on a renamed status doesn't break.

Let me know your thoughts?

// The status is eventually consistent with the request store and reconciled from the append-only request log.
rpc Status(StatusRequest) returns (StatusResponse) {}

// List returns request summaries for one queue whose lifecycles overlap a time window.

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.

I can't mentally parse the description :)

func (c *ListController) List(ctx context.Context, req *pb.ListRequest) (*pb.ListResponse, error) {
start := time.Now()
defer func() {
c.metricsScope.Timer("list_latency").Record(time.Since(start))

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.


if _, err := c.queueConfigs.Get(ctx, req.Queue); err != nil {
if errors.Is(err, queueconfig.ErrNotFound) {
return nil, errs.NewUserError(&UnrecognizedQueueError{Queue: req.Queue})

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.

req.Queue == ""
should also be a user error then? But it is not.
we need to design an error classification propagation for gateways/grpc, similar to orchestrator/queue

limit = defaultListPageSize
}
if limit > maxListPageSize {
limit = maxListPageSize

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.

I suggest to error instead

return base64.RawURLEncoding.EncodeToString(data), nil
}

func decodeListPageToken(raw string) (*listPageToken, error) {

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.

no need for a pointer


func decodeListPageToken(raw string) (*listPageToken, error) {
if raw == "" {
return nil, nil

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.

why is this not an error?

if !entity.IsKnownRequestStatus(status) {
return nil, fmt.Errorf("unknown status %q: %w", statusRaw, ErrInvalidRequest)
}
if _, ok := seen[status]; ok {

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.

nit: can always assign to a set, then after the iteration completes, transform to a sorted list.

return out
}

func equalStrings(a, b []string) bool {

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.

could use slices.Equal

@sbalabanov

Copy link
Copy Markdown
Contributor

user inpuit validation could be extracted into a separate function

@albertywu albertywu force-pushed the wua/list-api-summary-store branch from 788ef8d to 48dfb0b Compare July 7, 2026 22:29
@albertywu albertywu force-pushed the wua/list-api-gateway-rpc branch from d308ee7 to 27d33c8 Compare July 7, 2026 22:29
@albertywu albertywu requested a review from sbalabanov July 7, 2026 22:29
@albertywu albertywu changed the title [3/N] [List API] feat(gateway): expose request List RPC [4/N] [List API] feat(gateway): expose List API from request summaries Jul 7, 2026
@albertywu albertywu force-pushed the wua/list-api-summary-store branch from 48dfb0b to c0e7f96 Compare July 7, 2026 22:40
@albertywu albertywu force-pushed the wua/list-api-gateway-rpc branch from 27d33c8 to 12b84dc Compare July 7, 2026 22:40
@albertywu albertywu force-pushed the wua/list-api-summary-store branch from c0e7f96 to c82b235 Compare July 8, 2026 00:15
@albertywu albertywu force-pushed the wua/list-api-gateway-rpc branch from 12b84dc to 418983a Compare July 8, 2026 00:15
@albertywu albertywu force-pushed the wua/list-api-summary-store branch from c82b235 to d565bc9 Compare July 8, 2026 00:18
@albertywu albertywu force-pushed the wua/list-api-gateway-rpc branch from 418983a to 4010630 Compare July 8, 2026 00:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants