Skip to content

Commit 40195ca

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. **Work recorded after a request settles says so.** A build for a speculation path nobody needed any more can finish after its batch has landed and be recorded against every request in it — seen in a fifty-request run, where two rows carried a `building`/`built` pair timestamped 350ms after `landed`. Rendering those like any other event read as a landed change building itself afterwards, so a terminal status marks them `[after: …]`. The orchestrator does cancel unwanted builds, but only ones still running when it next polls, and the fake runner finishes instantly — so this is mostly a demo artifact that a real runner would usually cancel instead. **`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`. **`demo-requests` also takes the provider from the running stack.** The two have to agree, and nothing enforced it: a stack started with `PROVIDER=git` and a `make demo-requests` that was not told so mints fake changes pointing at no repository, which the git merger rejects as commits it cannot find. Observed as fifty consecutive failures reading `not available from remote origin`, with nothing in the error to say the provider was the problem. The stack knows which one it has — it is mounted at `/etc/submitqueue` — so a run given no provider of its own asks it, and one given a provider that disagrees says so before it starts rather than after fifty rejections. Finding the port is not enough on its own, because `?=` treats a variable exported in the shell as already set — so anyone who ran the `export GATEWAY_ADDR=…` the quickstart used to recommend keeps a dead port forever and never reaches the discovery at all. That is the failure this was meant to remove, so when the address came from the environment and a local stack is running somewhere else, both are named and `unset GATEWAY_ADDR` is suggested. An address given on the command line is deliberate and passes without comment. ## 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` - ✅ reproduced the provider mismatch that prompted the detection — a `PROVIDER=git` stack with a plain `make demo-requests` — and confirmed it now creates git changes and lands them, while an explicit `PROVIDER=fake` against that stack warns before starting - ✅ reproduced the stale-export case that prompted the guard — an exported address pointing at a port from an earlier stack, with a live stack elsewhere — and confirmed it names both and suggests unsetting, while the same address given on the command line stays silent - ✅ 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 40195ca

12 files changed

Lines changed: 800 additions & 40 deletions

File tree

Makefile

Lines changed: 66 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,55 @@ 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
116+
117+
# Resolves which provider to create changes for into $$provider, preferring the
118+
# one the running stack was started with.
119+
#
120+
# The two have to agree. A change minted for one provider is meaningless to a
121+
# stack wired to another: fake changes point at no repository, so a stack
122+
# running the git merger rejects every one of them as a commit it cannot find,
123+
# and fifty requests fail identically for a reason that is nowhere in the error.
124+
# The stack knows which provider it has — it is mounted at /etc/submitqueue —
125+
# so a run that was not told otherwise asks it rather than guessing.
126+
define resolve_provider
127+
provider="$(PROVIDER)"; \
128+
mounted=$$(docker inspect $(SUBMITQUEUE_LOCAL_PROJECT)-orchestrator-service-1 \
129+
--format '{{range .Mounts}}{{if eq .Destination "/etc/submitqueue"}}{{.Source}}{{end}}{{end}}' 2>/dev/null); \
130+
if [ -n "$$mounted" ]; then \
131+
running=$$(basename "$$mounted"); \
132+
if [ "$(origin PROVIDER)" = "file" ]; then \
133+
provider="$$running"; \
134+
elif [ "$$provider" != "$$running" ]; then \
135+
echo "Note: creating $$provider changes, but the running stack is '$$running'." >&2; \
136+
echo " They have to match — a $$provider change is not something a '$$running' stack can land." >&2; \
137+
fi; \
138+
fi
139+
endef
92140

93141
# Fails if git working tree is dirty. Usage: $(call assert_clean,fix command)
94142
define assert_clean
@@ -197,16 +245,17 @@ clean-proto: ## Clean generated proto files
197245
@echo "Proto clean complete!"
198246

199247
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 -- \
201-
-provider $(PROVIDER) \
248+
@set -e; $(resolve_gateway_addr); $(resolve_provider); \
249+
$(BAZEL) run //service/submitqueue/demo/requests -- \
250+
-provider $$provider \
202251
-repo $(DEMO_REPO) \
203252
-sandbox-dir $(SQ_GIT_SANDBOX_DIR) \
204253
-count $(COUNT) \
205254
-folders $(FOLDERS) \
206255
-files $(FILES) \
207256
-concurrency $(CONCURRENCY) \
208257
-stacked=$(STACKED) \
209-
-addr $(GATEWAY_ADDR) \
258+
-addr $$addr \
210259
-queue $(QUEUE) \
211260
-strategy $(STRATEGY) \
212261
-land=$(LAND) -watch=$(WATCH)
@@ -262,25 +311,29 @@ land: ## Land a change or a stack (PR=<url>, PRS="<url> <url>", or URI=<change-u
262311
echo " opts: QUEUE=$(QUEUE) STRATEGY=$(STRATEGY) GATEWAY_ADDR=$(GATEWAY_ADDR)"; \
263312
exit 2; \
264313
fi
265-
@$(BAZEL) run //service/submitqueue/gateway/client:gateway -- \
266-
-addr $(GATEWAY_ADDR) land \
314+
@set -e; $(resolve_gateway_addr); \
315+
$(BAZEL) run //service/submitqueue/gateway/client:gateway -- \
316+
-addr $$addr land \
267317
-queue $(QUEUE) \
268318
-strategy $(STRATEGY) \
269319
$(if $(PR),-pr $(PR)) $(foreach p,$(PRS),-pr $(p)) \
270320
$(if $(URI),-uri $(URI)) $(foreach u,$(URIS),-uri $(u))
271321

272322
land-status: ## Read a landed request's status (SQID=... [QUEUE=demo-queue])
273323
@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)
324+
@set -e; $(resolve_gateway_addr); \
325+
$(BAZEL) run //service/submitqueue/gateway/client:gateway -- \
326+
-addr $$addr status -queue $(QUEUE) -sqid $(SQID)
276327

277328
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)
329+
@set -e; $(resolve_gateway_addr); \
330+
$(BAZEL) run //service/submitqueue/gateway/client:gateway -- \
331+
-addr $$addr list -queue $(QUEUE) -since $(SINCE) -limit $(LIMIT)
280332

281333
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)
334+
@set -e; $(resolve_gateway_addr); \
335+
$(BAZEL) run //service/submitqueue/gateway/client:gateway -- \
336+
-addr $$addr watch -queue $(QUEUE) -since $(SINCE) -limit $(LIMIT)
284337

285338
license-fix: ## Add missing license headers to source files
286339
@$(BAZEL) run //tool/linter/licenseheader -- --fix
@@ -470,7 +523,7 @@ local-submitqueue-start: build-all-linux ## Start full stack (PROVIDER=fake|git|
470523
fi
471524
@echo ""
472525
@echo "Generate traffic with:"
473-
@echo " make demo-requests GATEWAY_ADDR=localhost:<gateway port>"
526+
@echo " make demo-requests"
474527

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

README.md

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,8 @@ Put traffic through the queue and watch it land. Requires Docker and Docker Comp
2121
# Start the full stack (Gateway + Orchestrator + Runway + MySQL)
2222
make local-submitqueue-start
2323

24-
# Compose publishes a random host port; the line above prints it, as does this
25-
make local-submitqueue-ps
26-
export GATEWAY_ADDR=localhost:<gateway port>
27-
28-
# Create changes, enqueue each as it is created, and watch them settle
24+
# Create changes, enqueue each as it is created, and watch them settle.
25+
# It finds the running stack's port and provider itself — nothing to copy.
2926
make demo-requests
3027

3128
# Stop services

doc/howto/DEVELOPMENT.md

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,15 +60,11 @@ docker ps
6060
# 2. Start the full stack
6161
make local-submitqueue-start
6262

63-
# 3. Read the gateway's port (Compose publishes a random one)
64-
make local-submitqueue-ps
65-
export GATEWAY_ADDR=localhost:<gateway port>
66-
67-
# 4. Create changes, enqueue them, and watch them land
63+
# 3. Create changes, enqueue them, and watch them land
6864
make demo-requests
6965

70-
# 5. Stop services
71-
make local-stop
66+
# 4. Stop services
67+
make local-submitqueue-stop
7268
```
7369

7470
[QUICKSTART.md](QUICKSTART.md) walks through the same run in detail, and on to `PROVIDER=git`, which lands real commits into a repository on disk — still with no credential.

doc/howto/QUICKSTART.md

Lines changed: 18 additions & 14 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:
@@ -178,13 +183,14 @@ Gateway gRPC port: 55295
178183
Merge target: /tmp/sq-sandbox/sandbox.git
179184
```
180185

181-
Then the same command as before, with the same `PROVIDER`:
186+
Then the same command as before, unchanged:
182187

183188
```bash
184-
export GATEWAY_ADDR=localhost:55295
185-
PROVIDER=git make demo-requests
189+
make demo-requests
186190
```
187191

192+
`demo-requests` creates changes for whichever provider the running stack was started with, so there is nothing to repeat and nothing to keep in sync. The two must agree — a fake change points at no repository, so a stack running the git merger rejects every one of them as a commit it cannot find — and rather than leaving that to memory, a run with no `PROVIDER` of its own asks the stack which one it has. Passing one that disagrees still works, and says so before it starts.
193+
188194
Now `demo-requests` pushes real branches with real commits, and landing them is a real cherry-pick and push. Look at the repository itself:
189195

190196
```bash
@@ -200,13 +206,11 @@ b5d86d6 seed the sandbox
200206

201207
The commits are there, and they are not the ones that were pushed: `SQUASH_REBASE` replays each change onto the target rather than merging it, which is why the queue can keep the trunk linear.
202208

203-
**`PROVIDER` has to match on both commands.** It selects what the stack merges with *and* what `demo-requests` creates; pointing fake changes at a stack wired to git means asking the merger to fetch a ref that was never pushed.
204-
205209
One property worth seeing, because it is the thing a submit queue exists for. A stack lands as a single push, so no reader ever observes it half-applied:
206210

207211
```bash
208212
git -C /tmp/sq-sandbox/sandbox.git reflog show refs/heads/main | wc -l
209-
PROVIDER=git make demo-requests COUNT=3 STACKED=true
213+
make demo-requests COUNT=3 STACKED=true
210214
git -C /tmp/sq-sandbox/sandbox.git reflog show refs/heads/main | wc -l
211215
```
212216

@@ -257,7 +261,7 @@ PROVIDER=github make local-submitqueue-start
257261

258262
The token is required rather than defaulted: a stack that silently falls back to the fake integrations reports changes as landed without having gone near the provider, which is a much worse way to find out.
259263

260-
From here everything is as before — `PROVIDER=github make demo-requests` opens real pull requests, enqueues them and watches them land.
264+
From here everything is as before — `make demo-requests` opens real pull requests, enqueues them and watches them land, having picked up from the running stack that this one is GitHub.
261265

262266
### Land a pull request
263267

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)