feat(stovepipe): add the build stage controller#407
Conversation
| return fmt.Errorf("BuildController failed to publish build signal for %s: %w", build.ID, err) | ||
| } | ||
|
|
||
| c.logger.Infow("triggered build", |
There was a problem hiding this comment.
do we need this as Info log? or should it be debug?
There was a problem hiding this comment.
it matches convention for one-shot per-build events elsewhere (in sq orchestrator build step). my preference is reserving Debug for high-frequency logs like buildsignal's poll loop?
There was a problem hiding this comment.
Agree with keeping in Info for now, it would be nice to be able to scroll through all the logs of builds that got triggered and frequency of these shouldn't be unreasonably high.
There was a problem hiding this comment.
eventually it's a problem for ratining logs over time when they become too much due to all the info logs
There was a problem hiding this comment.
ok then shall we do same change for submitqueue orchestrator build as well?
| if errors.Is(err, storage.ErrNotFound) { | ||
| return entity.Request{}, errs.NewRetryableError(fmt.Errorf("request %s not found yet: %w", id, err)) | ||
| } |
There was a problem hiding this comment.
why would request be not found?
There was a problem hiding this comment.
Shouldn't happen in the common case as per we discussed in rfc (#336 (comment)). I kept it as a defensive guard but it's also making sense to remove here...
There was a problem hiding this comment.
I replied on the other one - I think the defensive guard is ok here... it could be missing due to replication lag if this task hits a different replica than the one that inserted the row.
There was a problem hiding this comment.
i think we should not add that logic here, we likely going to pin to primary region and require a consistent data store for this
3 participants
Why?
Stovepipe needs a controller that actually triggers builds using the BuildRunner contract.
What?
Adds
stovepipe/controller/build.Processunmarshals aBuildRequest(request id only), reloads theRequest, no-ops if already terminal, resolves the runner viaFactory.For(Config{QueueName}), reads the already-decidedBuildStrategy/BaseURI(retryable if not yet visible — never re-derives full-vs-incremental itself), triggers viabuildRunner.Trigger(ctx, baseURI, request.URI, nil), persists aBuildrow (BuildStore.Create, tolerant ofErrAlreadyExists), and publishes the resulting build id to thebuildsignaltopic, partitioned by build id.Test Plan
stovepipe/controller/build/build_test.gocovers: terminal-request no-op, unresolved build strategy (retryable),Factory.For/Trigger/BuildStore.Create/publish failure paths, and the happy path for both full and incremental strategies.go build ./...andgo test ./stovepipe/controller/build/...pass.Issue