Ux 1341 remove rest for topic listing#2513
Conversation
Extend ListTopicsResponse.Topic with cleanup_policy and an aggregated LogDirSummary (total size, per-broker replica errors, hint) so the topic list UI can be served entirely from the gRPC endpoint, removing the need for the REST /topics call. Regenerate protogen and OpenAPI artifacts.
Delete useLegacyListTopicsQuery and its REST-shaped mapper; migrate all consumers to the native gRPC useListTopicsQuery (ListTopicsResponse_Topic). Callers needing the full list pass pageSize -1, matching prior behavior. - useListTopicsQuery now forwards staleTime/refetchOnWindowFocus (narrowed to primitives to keep useQuery generic inference intact). - topic-list pages drop the dead allowedActions permission popover (never populated by the old mapper) and switch the name column id to 'name'. - renderLogDirSummary accepts number | bigint and an optional summary so it serves both the REST and gRPC topic shapes without a mapper. - Update tests to the native gRPC shape.
|
The latest Buf updates on your PR. Results from workflow Buf CI / validate (pull_request).
|
…ic-step test - buf.yml: buf-action defaults to the latest buf, whose formatter collapses single-field messages and flags ~40 already-formatted protos. Pin to 1.65.0 (the repo's canonical version from taskfiles/proto.yaml) so CI matches `task proto:generate`. - add-topic-step.test.tsx: topics now load via the gRPC ListTopics query over the test's router transport, not config.fetch. Register a listTopics handler (with a topicNames override) and drop the dead mockFetch-based seeding and the `expect(mockFetch).toHaveBeenCalled()` assertions that could never pass.
…apper Add TestTopicSummariesToProto guarding the gRPC ListTopics response shape: cleanup policy and the aggregated log dir summary (size, hint, per-broker replica errors) must survive mapping from console.TopicSummary. There was no backend coverage for ListTopics, so a regression showed up only as empty columns in the UI.
c-julin
left a comment
There was a problem hiding this comment.
Not to be a pain but this is a change we dont want to make on the dataplane api :( adding log dirs. Its been a long time ticket which i haven't done yet to split this out, if we can wait until next week i'll try prioritise it
| // Topic cleanup policy (e.g. `delete`, `compact`). | ||
| string cleanup_policy = 5; | ||
| // Aggregated log directory size for the topic across all replicas. | ||
| LogDirSummary log_dir_summary = 6; | ||
| } | ||
| // LogDirSummary describes the aggregated on-disk size of a topic. | ||
| message LogDirSummary { | ||
| // Total size of the topic in bytes across all replicas. | ||
| int64 total_size_bytes = 1; | ||
| // Errors encountered while querying log dirs from individual brokers. | ||
| repeated ReplicaError replica_errors = 2; | ||
| // Optional human-readable hint explaining a partial or missing result. | ||
| string hint = 3; | ||
| } | ||
| // ReplicaError describes a log dir query error for a single broker. | ||
| message ReplicaError { | ||
| // Broker that returned the error. | ||
| int32 broker_id = 1; | ||
| // Error message returned by the broker. | ||
| string error = 2; |
There was a problem hiding this comment.
can we make this proto have more information, so it's properly validated, maybe CEL expressions too, and using correct types like cleanup_policy I imagine can be more strict than a string
@c-julin no problem feel free to update me or we can get in touch in DM |
Conflict resolutions: - .github/workflows/buf.yml: take master's buf 1.71.0 (supersedes the 1.65.0 pin, which existed to match BUF_VERSION in taskfiles/proto.yaml — master bumped both). - topic-list-new.tsx: combine both sides. DropdownMenuTrigger uses master's Base UI `render` prop with this branch's gRPC `topic.name` field. TopicName keeps this branch's version — the permission popover is dead code, as gRPC ListTopics does not return allowedActions. - proto/gen/openapi/openapi.json: regenerated via `task proto:generate-openapi3` (single-line generated file, textual merge is meaningless). Also ports sql-workspace.tsx (new on master) from the deleted REST hook useLegacyListTopicsQuery to useListTopicsQuery. Git merged this file cleanly, but it referenced a hook this branch removes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
✅ Clean — no registry drift, off-token colours, or ad-hoc classesApp:
Generated by lookout audit-changes. |
The topic list renders cleanup policy and on-disk size, which the earlier commits added to the dataplane ListTopics response and switched its handler to console.GetTopicsOverview. That made the DescribeLogDirs broker fan-out run on every external call to the public GET /v1/topics, and put Console-only fields into a versioned, breaking-change-guarded, BSR-published contract. Revert the dataplane API to master and add a Console-facing console.v1alpha1.TopicService.ListTopics instead: it wraps the dataplane request but returns a Console-specific response carrying cleanup policy and the log dir summary. This follows the existing wrapper precedent (transform, security, pipeline, secret, shadowlink) — Connect-only, no REST/OpenAPI, exempt from breaking checks. Unlike those wrappers it calls GetTopicsOverview directly rather than delegating to the dataplane handler, since the dataplane handler no longer returns the enriched data by design. Frontend useListTopicsQuery now targets the Console service; its external signature is unchanged so call sites only swap the topic type import. createTopic stays on the dataplane API. Addresses PR review: log dirs must not land on the dataplane API. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
No description provided.