Skip to content

Commit e3ce570

Browse files
committed
feat(client): scrollable watch, and a gateway address nobody has to copy
## Summary ### Why? Two things made a watch awkward to actually use. **A big table could only be trimmed.** The previous change stopped a frame taller than the window from repainting the screen, by dropping settled rows and saying how many it had dropped. That keeps the redraw honest but it is still a table you cannot read: the rows are there, and the only reason they are not on screen is that the renderer had to choose. What a reader wants is what `top` gives them — the whole table, and a way to move through it. **The gateway address had to be copied by hand, and went stale.** Compose publishes a fresh random port on every start, so a `GATEWAY_ADDR` noted from an earlier run points at a port that no longer exists, and every demo command fails with a connection refused that says nothing about why. That is not a hypothetical: it is the most common way these commands fail. ### What? **A watch of a queue is now a scrollable full-screen view.** It takes the alternate buffer while it runs, reads keys in raw mode, and gives the screen back untouched afterwards: | Key | | |---|---| | `↑` `↓` / `k` `j` | one row | | `PgUp` `PgDn` / `Space` | one screen | | `g` `G` | first row, last row | | `q` | stop watching | It follows the end of the table by default, so rows and stages appear without anyone touching it; scrolling up holds the reader's place, and scrolling back to the bottom resumes following. There is no dedicated key for that, because being at the end is what following is. The full-screen view also removes the class of bug the trimming worked around, rather than managing it: the alternate buffer never scrolls, so each frame is painted from the top and there is no previous frame to find. The trimming path remains for the case where it is still needed — a terminal on stdout but not on stdin, where there is a screen to draw on but nobody to press a key. The finished table is printed into the restored screen whole, however tall it is. Nothing is drawn over it, so a long one scrolls, which is what a reader of a completed run wants. No new dependency: `golang.org/x/term` was already in use for the window size and provides raw mode too. **`make land`, `land-status`, `land-list`, `land-watch` and `demo-requests` find the gateway themselves**, by asking Docker for the running stack's published port. `GATEWAY_ADDR` is now an override for reaching a gateway the Makefile did not start, and a stack that is not running produces a sentence saying so rather than a refused connection. The resolution is done inside each recipe rather than as a `$(shell ...)` assignment, which would shell out to Docker on every `make help`. ## Test Plan - ✅ drove the view through a pty with real keystrokes — `G`, two up-arrows, `PgUp`, `q` — and read the positions back out of the footer: `40-40 → 39-40 → 38-40 → 36-40 of 40`, then a clean exit - ✅ the alternate screen is entered once and left once in every run captured, so the terminal is never left on it - ✅ after `q`, all 40 rows are printed into the restored screen; after a settled 25-request run, all 25 are, with nothing hidden - ✅ `make demo-requests` and `make land-list` with no `GATEWAY_ADDR` set at all; with it set explicitly; and with no stack running, which now says `No gateway found: 'submitqueue' is not running` - ✅ redirected output still produces a plain log: `make demo-requests LAND=false` and piped runs take neither the screen nor the keyboard - ✅ new tests for the parts that are not a terminal: every key and escape sequence including one split across reads, and the scroll arithmetic — bounds, paging, and that scrolling up releases follow while rows arriving do not move a view that has scrolled away - ✅ `make test` (105 targets), `make lint`, `make gazelle` Two behaviours worth knowing. A bare `Escape` is not acted on until another key follows, and swallows it — the alternative is misreading an arrow whose bytes arrive in separate reads, which is worse and intermittent. And with tall wrapped rows in a short window, moving up one row can land back at the bottom, because the number of rows that fit changes with their height.
1 parent b8a1ed9 commit e3ce570

9 files changed

Lines changed: 740 additions & 19 deletions

File tree

Makefile

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,31 @@ LAND ?= true
8888
WATCH ?= true
8989
QUEUE ?= demo-queue
9090
STRATEGY ?= SQUASH_REBASE
91-
GATEWAY_ADDR ?= localhost:8081
91+
# Where the client looks for the gateway. Left empty, every target below finds
92+
# the running stack's published port for itself — Compose picks a fresh one on
93+
# every start, and a number copied out of a previous run's output is the most
94+
# common reason a demo command cannot connect. Set it to reach a gateway this
95+
# Makefile did not start.
96+
GATEWAY_ADDR ?=
97+
98+
# Resolves $(GATEWAY_ADDR), or the local stack's port when it is unset, into
99+
# $$addr for the recipe that includes it. Not a $(shell ...) assignment: that
100+
# would run at parse time, shelling out to Docker on every `make help`.
101+
define resolve_gateway_addr
102+
addr="$(GATEWAY_ADDR)"; \
103+
port=$$(docker port $(SUBMITQUEUE_LOCAL_PROJECT)-gateway-service-1 8080 2>/dev/null | head -1 | sed 's/.*://'); \
104+
if [ -z "$$addr" ]; then \
105+
if [ -z "$$port" ]; then \
106+
echo "No gateway found: '$(SUBMITQUEUE_LOCAL_PROJECT)' is not running." >&2; \
107+
echo "Start it with 'make local-submitqueue-start', or name one with GATEWAY_ADDR=host:port." >&2; \
108+
exit 2; \
109+
fi; \
110+
addr="localhost:$$port"; \
111+
elif [ "$(origin GATEWAY_ADDR)" = "environment" ] && [ -n "$$port" ] && [ "$$addr" != "localhost:$$port" ]; then \
112+
echo "Note: GATEWAY_ADDR=$$addr is exported in your shell, so that is what will be used." >&2; \
113+
echo " The stack running here is on localhost:$$port — 'unset GATEWAY_ADDR' to use it." >&2; \
114+
fi
115+
endef
92116

93117
# Fails if git working tree is dirty. Usage: $(call assert_clean,fix command)
94118
define assert_clean
@@ -197,7 +221,8 @@ clean-proto: ## Clean generated proto files
197221
@echo "Proto clean complete!"
198222

199223
demo-requests: ## Create N changes, enqueue each as it is created, and watch (PROVIDER=fake|git|github COUNT=3 FOLDERS=0 FILES=3 CONCURRENCY=5)
200-
@$(BAZEL) run //service/submitqueue/demo/requests -- \
224+
@set -e; $(resolve_gateway_addr); \
225+
$(BAZEL) run //service/submitqueue/demo/requests -- \
201226
-provider $(PROVIDER) \
202227
-repo $(DEMO_REPO) \
203228
-sandbox-dir $(SQ_GIT_SANDBOX_DIR) \
@@ -206,7 +231,7 @@ demo-requests: ## Create N changes, enqueue each as it is created, and watch (PR
206231
-files $(FILES) \
207232
-concurrency $(CONCURRENCY) \
208233
-stacked=$(STACKED) \
209-
-addr $(GATEWAY_ADDR) \
234+
-addr $$addr \
210235
-queue $(QUEUE) \
211236
-strategy $(STRATEGY) \
212237
-land=$(LAND) -watch=$(WATCH)
@@ -262,25 +287,29 @@ land: ## Land a change or a stack (PR=<url>, PRS="<url> <url>", or URI=<change-u
262287
echo " opts: QUEUE=$(QUEUE) STRATEGY=$(STRATEGY) GATEWAY_ADDR=$(GATEWAY_ADDR)"; \
263288
exit 2; \
264289
fi
265-
@$(BAZEL) run //service/submitqueue/gateway/client:gateway -- \
266-
-addr $(GATEWAY_ADDR) land \
290+
@set -e; $(resolve_gateway_addr); \
291+
$(BAZEL) run //service/submitqueue/gateway/client:gateway -- \
292+
-addr $$addr land \
267293
-queue $(QUEUE) \
268294
-strategy $(STRATEGY) \
269295
$(if $(PR),-pr $(PR)) $(foreach p,$(PRS),-pr $(p)) \
270296
$(if $(URI),-uri $(URI)) $(foreach u,$(URIS),-uri $(u))
271297

272298
land-status: ## Read a landed request's status (SQID=... [QUEUE=demo-queue])
273299
@if [ -z "$(SQID)" ]; then echo "Usage: make land-status SQID=demo-queue/1 [QUEUE=demo-queue]"; exit 2; fi
274-
@$(BAZEL) run //service/submitqueue/gateway/client:gateway -- \
275-
-addr $(GATEWAY_ADDR) status -queue $(QUEUE) -sqid $(SQID)
300+
@set -e; $(resolve_gateway_addr); \
301+
$(BAZEL) run //service/submitqueue/gateway/client:gateway -- \
302+
-addr $$addr status -queue $(QUEUE) -sqid $(SQID)
276303

277304
land-list: ## Show a queue's recent requests as a table (QUEUE=demo-queue SINCE=1h LIMIT=50)
278-
@$(BAZEL) run //service/submitqueue/gateway/client:gateway -- \
279-
-addr $(GATEWAY_ADDR) list -queue $(QUEUE) -since $(SINCE) -limit $(LIMIT)
305+
@set -e; $(resolve_gateway_addr); \
306+
$(BAZEL) run //service/submitqueue/gateway/client:gateway -- \
307+
-addr $$addr list -queue $(QUEUE) -since $(SINCE) -limit $(LIMIT)
280308

281309
land-watch: ## Follow a queue's requests until they settle (QUEUE=demo-queue SINCE=15m LIMIT=50)
282-
@$(BAZEL) run //service/submitqueue/gateway/client:gateway -- \
283-
-addr $(GATEWAY_ADDR) watch -queue $(QUEUE) -since $(SINCE) -limit $(LIMIT)
310+
@set -e; $(resolve_gateway_addr); \
311+
$(BAZEL) run //service/submitqueue/gateway/client:gateway -- \
312+
-addr $$addr watch -queue $(QUEUE) -since $(SINCE) -limit $(LIMIT)
284313

285314
license-fix: ## Add missing license headers to source files
286315
@$(BAZEL) run //tool/linter/licenseheader -- --fix
@@ -470,7 +499,7 @@ local-submitqueue-start: build-all-linux ## Start full stack (PROVIDER=fake|git|
470499
fi
471500
@echo ""
472501
@echo "Generate traffic with:"
473-
@echo " make demo-requests GATEWAY_ADDR=localhost:<gateway port>"
502+
@echo " make demo-requests"
474503

475504
local-submitqueue-stop: ## Stop the SubmitQueue stack (keeps data and PROVIDER=git's sandbox)
476505
@echo "Stopping SubmitQueue services..."

doc/howto/QUICKSTART.md

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,7 @@ Compose publishes each service on a **random** host port so several stacks can r
2828
Gateway gRPC port: 58537
2929
```
3030

31-
Export it, because every command below needs it:
32-
33-
```bash
34-
export GATEWAY_ADDR=localhost:58537
35-
```
36-
37-
Leaving it unset does not fall back to anything useful — the client's default is `localhost:8081`, the `go run` port rather than the compose one.
31+
You do not have to note it down. Every command below finds the running stack's port for itself, which matters because Compose picks a fresh one on every start — a number copied from an earlier run is the most common reason a demo command cannot connect. Set `GATEWAY_ADDR=host:port` only to reach a gateway this Makefile did not start.
3832

3933
## Put traffic through it
4034

@@ -118,6 +112,17 @@ Eight builds means the batch was speculating down eight paths at once, and `wait
118112

119113
`land-watch` fixes its set when it starts and exits non-zero if any request in that set finishes anywhere other than `landed`, which makes it usable from a script. A request accepted after the watch begins is not picked up: a watch that grew as the queue did would never finish.
120114

115+
Watching more requests than the window holds takes over the screen while it runs, the way `top` does, so the table can be scrolled rather than trimmed:
116+
117+
| Key | |
118+
|---|---|
119+
| `` `` or `k` `j` | one row |
120+
| `PgUp` `PgDn` or `Space` | one screen |
121+
| `g` `G` | first row, last row |
122+
| `q` | stop watching |
123+
124+
The view follows the end of the table by default, so new rows and new stages appear without touching it. Scrolling up holds your place; scrolling back to the bottom starts following again. The screen you had is restored on exit and the finished table is printed into it whole, so nothing is lost with the view — and when output is redirected, none of this happens at all and the run stays a plain log.
125+
121126
A listing of a busy queue is mostly `speculating` rows, since that is where a request spends most of its active life — waiting on the build its batch was admitted for.
122127

123128
Under the hood these are `client list` and `client watch`, which take a queue and reach any gateway:

service/submitqueue/demo/requests/main.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,11 +211,21 @@ func run(ctx context.Context, cfg config) error {
211211
return nil
212212
}
213213

214+
// A large run has more changes than a window has lines, so the wait happens
215+
// in a full-screen view the reader can scroll. Restored before Conclude, so
216+
// the final table lands in the scrollback and not on a screen that is about
217+
// to be handed back.
218+
stop, quit := t.Interact(ctx)
219+
defer stop()
220+
214221
select {
215222
case <-ctx.Done():
223+
stop()
216224
return ctx.Err()
225+
case <-quit:
217226
case <-t.Settled():
218227
}
228+
stop()
219229
return t.Conclude()
220230
}
221231

service/submitqueue/gateway/client/main.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,13 +282,23 @@ func runWatch(ctx context.Context, sq *client.Client, args []string) error {
282282
t.Seal()
283283
t.Note("watching %d request(s) in %s", len(rows), *queue)
284284

285+
// A watch of a busy queue holds more requests than a window does, so it runs
286+
// as a full-screen view the reader can scroll. Restored before Conclude, so
287+
// the final table lands in the scrollback rather than disappearing with the
288+
// screen it was drawn on.
289+
stop, quit := t.Interact(ctx)
290+
defer stop()
291+
285292
go t.Poll(ctx, sq.Gateway(), *queue)
286293

287294
select {
288295
case <-ctx.Done():
296+
stop()
289297
return ctx.Err()
298+
case <-quit:
290299
case <-t.Settled():
291300
}
301+
stop()
292302
return t.Conclude()
293303
}
294304

submitqueue/client/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ go_library(
66
"conn.go",
77
"land.go",
88
"query.go",
9+
"tui.go",
910
"view.go",
1011
"watch.go",
1112
],
@@ -28,6 +29,7 @@ go_test(
2829
srcs = [
2930
"conn_test.go",
3031
"query_test.go",
32+
"tui_test.go",
3133
"view_test.go",
3234
],
3335
embed = [":go_default_library"],

0 commit comments

Comments
 (0)