feat(stovepipe): extend Request entity to support new process step#319
feat(stovepipe): extend Request entity to support new process step#319mnoah1 wants to merge 2 commits into
Conversation
mnoah1
changed the title
| // BaseURI is the base URI to be used for this request when the strategy is incremental. | ||
| // Empty for full-monorepo builds and cold start. | ||
| BaseURI string `json:"base_uri"` |
There was a problem hiding this comment.
do we need this in request itself? would we publish to build only when the last one is done? i guess question i have is where to we decide latest last green?
There was a problem hiding this comment.
where to we decide latest last green?
We currently propose doing this just before starting builds (process steps - 7a).
do we need this in request itself?
Once selected above, we need to store it somewhere for retrieval once the builds start. I think Request is reasonable for the MVP, but alternatively we could put it in a separate table of planned builds. The latest green value stored in the Queue entity represents more of a moving bookmark of latest green, while the value here serves as the fixed base for that specific request.
would we publish to build only when the last one is done?
Yes - when the concurrent builds is 1, that is effectively true. When increased, it would be whenever an available build slot has become free.
| // requestCounter extracts the per-queue counter suffix from a request id. | ||
| func requestCounter(id, queue string) (int64, error) { | ||
| prefix := requestIDPrefix(queue) | ||
| if !strings.HasPrefix(id, prefix) { | ||
| return 0, fmt.Errorf("request id %q does not match queue %q", id, queue) | ||
| } | ||
| return strconv.ParseInt(id[len(prefix):], 10, 64) | ||
| } |
There was a problem hiding this comment.
do we need to understand the format the ID? can we treat it string which is separate from seq?
There was a problem hiding this comment.
I was going back and forth a bit on this (initially I was thinking we'll track sequence and ID as two separate fields). But as I worked on this branch, realized we could avoid a separate sequence field if we require sequence info to be encoded in the ID and use a comparison function. Do you have any thoughts on one approach vs. the other? Obviously there is the cost to parse the int on each comparison, but doing that a handful of times per request will be far from a bottleneck.
Align entity and storage contract tests with review feedback to use "full" instead of "monorepo" for the whole-repo build strategy.
mnoah1
force-pushed
the
mnoah1/stovepipe-request-entity
branch
from
bbf0abf to
90cbeef
Compare
2 participants
What?
Updating the Request entity to accommodate upcoming functionality in the process step:
RequestStatevalues -processingonce the processing step has started, andsupersededfor requests that will be skipped in favor of building a newer oneCompareRequestIDfunction for cases that require ID comparison, and converge to just storing the ID.Why?
Implementing behavior in https://github.com/uber/submitqueue/blob/main/doc/rfc/stovepipe/steps/process.md - which introduces two new outcome states and includes logic to determine the build strategy for a request.
Test Plan