feat(stovepipe): add commit ingestion abstractions by gsturges · Pull Request #200 · uber/submitqueue

gsturges · 2026-06-04T23:38:53Z

Summary

Add entity.PushEvent to represent a branch push (repo, branch, SHAs, pusher) for webhook/Kafka-driven ingestion.
Define CommitIngester and PushEventHandler in stovepipe/extension so a consumer can subscribe to push events and dispatch work.
Add stovepipe/core/filter with Config / ShouldProcess to gate processing by watched repo and optional branch list (empty list = all branches for that repo).
Add commitingester.LoggingHandler as a no-op PushEventHandler stub until persistence/pipeline wiring lands.

Test plan

make test (or targeted tests if added in a follow-up)
make lint

Made with Cursor

CLAassistant · 2026-06-04T23:39:00Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

behinddwalls · 2026-06-05T01:35:38Z

+
+// PushEvent represents a git push to a branch detected from a webhook event.
+// Fields are immutable after construction.
+type PushEvent struct {


do we need to be git aware? @mnoah1

SQ currently is not git aware in general for most part, so my general thought here would be to keep that semantics similar and define abstractions that can allow us to work with git or some other vcs, specific impl could be left to the implementer to decide

This case seems very close to SubmitQueue's ChangeInfo struct, so maybe we could leverage that:

Internal extension subscribes to the events and translates them into a shape similar to ChangeInfo - maybe even reuse the existing entity altogether.

Similarly to the github.com:// schema, add a raw git:// schema that identifies the path to the change on a git remote or other vcs - at this point, it doesn't matter which code review system was used, just where to find the change on remote.

Will work well as an identifier in the db like the existing change table - easy to provide two uri's and compare them.

@behinddwalls does this make sense?

Yes, this seems fine. We can think about whether to re-use the Change entity...we might have to move it out of SQ domain and into the top level

mnoah1 · 2026-06-05T22:33:17Z

+// Example:    "git://github.com/uber/go-code/refs/heads/main/c3a4d5e6f789..."
+//
+// Fields are immutable after construction.
+type CommitEvent struct {


Let's use the ChangeInfo naming from our earlier chat (and can use Change similarly throughout the PR where it says Commit currently).

mnoah1 · 2026-06-05T22:36:48Z

+
+	"github.com/uber/submitqueue/stovepipe/entity"
+)
+


Add a New() function to enforce interface compliance here (and you can accept the logger as a param)

mnoah1 · 2026-06-05T22:37:53Z

+func (LoggingHandler) HandlePushEvent(ctx context.Context, event entity.PushEvent) error {
+	// Caller (the Kafka consumer in go-code) is responsible for actual logging.
+	// This stub is intentionally a no-op at the OSS layer.
+	return nil


Add an actual log line here - once we get this wired into the internal service we can deploy and confirm it emits real events.

mnoah1 · 2026-06-05T22:39:31Z

+
+// CommitEventHandler processes a single commit event received from the ingester.
+type CommitEventHandler interface {
+	HandleCommitEvent(ctx context.Context, event entity.CommitEvent) error


I think naming has diverged a bit here - we have HandleCommitEvent, HandlePushEvent. Let's go with something that mirrors our "Change" terminology - how about IngestChange

behinddwalls · 2026-06-06T14:51:00Z

+	// URI is the canonical VCS identifier for this change.
+	// Scheme is "git://"; path encodes host, repo, ref, and new revision.
+	// This mirrors the ChangeInfo.URI pattern used in SubmitQueue.
+	URI string `json:"uri"`


a single push event can have multiple commits?

@behinddwalls that is how I interpreted the ERD saying that batches of commits will be validated. @mnoah1 has this changed?

behinddwalls · 2026-06-06T14:52:01Z

+	// PreviousURI is the URI of the prior revision on the same ref, if known.
+	// Empty string if unavailable.
+	// Example: "git://github.com/uber/go-code/refs/heads/main/aabbccdd..."
+	PreviousURI string `json:"previous_uri,omitempty"`
+
+	// Author is the identity of the person who authored the change.
+	Author Author `json:"author"`


do we care about these? I am wondering if we can just promote the Change entity from submitqueue to the top and use the same here?

@behinddwalls I am keep consistent with what I saw in submitqueue/entity. Otherwise, there's no strong reason to pull it out into its own type — it adds indirection without adding clarity.

behinddwalls · 2026-06-17T06:51:11Z

+// start topic. A trunk change is one commit, so the event carries one git-backed URI. It is
+// source-agnostic: both the webhook and the reconciliation poller emit it. Additional fields
+// (e.g. source, committer time) can be added later as ingestion needs them.
+type ChangeEvent struct {


maybe we start from the proto for gateway what will be the shape of Request and Response for API, in general ChangeEvent is not needed as Change entity captures this already

Introduce ChangeInfo, ChangeIngester/ChangeHandler, a no-op logging handler stub, and URI watch filtering for downstream ingestion.

Co-authored-by: Preetam Dwivedi <behinddwalls@gmail.com>

Add Commit and Batch as the core domain entities the validation pipeline operates on, each with status types and optimistic-locking Version fields. Add CommitStore and BatchStore extension interfaces scoped to their respective service owners (gateway and orchestrator). Fix missing BUILD.bazel files for the filter and changeingester packages. Flatten ChangeInfo.Author into AuthorName/AuthorEmail fields directly.

Replace ChangeInfo with ChangeEvent as the ingestion payload type and introduce ChangeURI as the lightweight pipeline reference. Update Commit entity and create CommitStatusentity. Update ChangeHandler and the filter to accept ChangeEvent. Remove the storage extension and Batch entity, which belong to the orchestrator.

gsturges requested review from a team, behinddwalls and sbalabanov as code owners June 4, 2026 23:38

behinddwalls reviewed Jun 5, 2026

View reviewed changes

behinddwalls requested changes Jun 5, 2026

View reviewed changes

gsturges force-pushed the gsturges/codem-101-stovepipe-commit-ingestion-abstractions branch 3 times, most recently from 64a6c14 to 9bbb8f9 Compare June 5, 2026 22:20

mnoah1 reviewed Jun 5, 2026

View reviewed changes

gsturges force-pushed the gsturges/codem-101-stovepipe-commit-ingestion-abstractions branch from 9bbb8f9 to 9bfce3e Compare June 5, 2026 23:22

mnoah1 approved these changes Jun 6, 2026

View reviewed changes

behinddwalls reviewed Jun 6, 2026

View reviewed changes

gsturges force-pushed the gsturges/codem-101-stovepipe-commit-ingestion-abstractions branch 2 times, most recently from 4dda171 to 0239b4a Compare June 10, 2026 00:44

gsturges force-pushed the gsturges/codem-101-stovepipe-commit-ingestion-abstractions branch 2 times, most recently from 21bd79f to eadd973 Compare June 16, 2026 23:07

behinddwalls reviewed Jun 17, 2026

View reviewed changes

gsturges force-pushed the gsturges/codem-101-stovepipe-commit-ingestion-abstractions branch from eadd973 to 8b2ce3e Compare June 17, 2026 17:12

gsturges and others added 3 commits June 17, 2026 11:29

feat(stovepipe): add change ingestion hooks and push filtering

0a01197

Introduce ChangeInfo, ChangeIngester/ChangeHandler, a no-op logging handler stub, and URI watch filtering for downstream ingestion.

Apply suggestion from @behinddwalls

2143206

Co-authored-by: Preetam Dwivedi <behinddwalls@gmail.com>

gsturges force-pushed the gsturges/codem-101-stovepipe-commit-ingestion-abstractions branch from 8b2ce3e to 95e8d29 Compare June 17, 2026 18:30

gsturges marked this pull request as draft June 17, 2026 18:38

gsturges force-pushed the gsturges/codem-101-stovepipe-commit-ingestion-abstractions branch from 95e8d29 to 3e26387 Compare June 17, 2026 18:44

Conversation

gsturges commented Jun 4, 2026

Summary

Test plan

Uh oh!

CLAassistant commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mnoah1 Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

CLAassistant commented Jun 4, 2026 •

edited

Loading

mnoah1 Jun 5, 2026 •

edited

Loading