Skip to content

Refactor Treeherder's branch model#9496

Open
ahal wants to merge 5 commits into
mozilla:masterfrom
ahal:refactor_branch_model
Open

Refactor Treeherder's branch model#9496
ahal wants to merge 5 commits into
mozilla:masterfrom
ahal:refactor_branch_model

Conversation

@ahal
Copy link
Copy Markdown
Member

@ahal ahal commented May 7, 2026

This makes backend changes to better support complex mappings between projects and repositories / branches:

  1. Replace comma-separated list of branches with proper join table in the db
  2. Add support for processing any arbitrary branch
  3. Replace the "pull request" magic string with a new model field

@ahal ahal force-pushed the refactor_branch_model branch from 4f9489f to fa96c6e Compare May 8, 2026 17:17
@ahal
Copy link
Copy Markdown
Member Author

ahal commented May 8, 2026

Alright, a couple notes for the reviewer:

  1. Yes this is Claude generated, but I took the time to split it out into small atomic commits and have a reasonable understanding of what it's doing (insofar as someone not familiar with Treeherder is able).
  2. I successfully tested this against my local docker-compose instance (both that old branches didn't break and branches using the new features). But we should probably do some extensive testing against staging as well. I'm not sure what the process is for that.
  3. I recommend reviewing commit by commit
  4. The "pull request" and RepositoryBranch commits could in theory be dropped and I'd still be able to proceed without them. Initially I thought I needed to do those things, but later realized I could get by without them. I think they're worth taking, but if there's some reason we shouldn't, I'm open to dropping them.
  5. I have a couple follow-up commits that add Push.branch info to the API and UI. I decided to leave this out of this PR as it was too orthogonal to everything else. But if this lands as-is, I'll submit them later.

@ahal ahal marked this pull request as ready for review May 8, 2026 17:24
@ahal ahal requested a review from a team as a code owner May 8, 2026 17:24
@ahal ahal force-pushed the refactor_branch_model branch from fa96c6e to 43aaae6 Compare May 8, 2026 17:40
@Archaeopteryx
Copy link
Copy Markdown
Collaborator

The changes will be applied to the prototype instance in the next 30 minutes.

@Archaeopteryx
Copy link
Copy Markdown
Collaborator

Deployment to prototype failed with:

psycopg2.errors.UniqueViolation: duplicate key value violates unique constraint "repository_branch_repository_id_branch_a18d8a40_uniq"
DETAIL:  Key (repository_id, branch)=(47, master) already exists.

@ahal
Copy link
Copy Markdown
Member Author

ahal commented May 13, 2026

@Archaeopteryx so the problem is that migrations/0049_repository_branch_model.py converts existing branch values into new RepositoryBranch records.

Then load_initial_data.py attempts to insert them again based on the repository_branch.json fixture, resulting in the duplicate error.

I need to understand how the deploys work to figure out the proper fix. Does load_initial_data.py always get called on deploy? E.g, even on deploys to production?

If so, then I think the migration just needs to not convert existing branches to RepositoryBranch records and we can let the fixture do its thing.

If not, Claude suggests we add a line to load_initial_data.py that deletes all RepositoryBranch objects prior to re-loading them.

Wdyt?

@ahal
Copy link
Copy Markdown
Member Author

ahal commented May 13, 2026

Possibly answered my own question. Assuming this is what runs during migrations:
https://github.com/mozilla/treeherder/blob/master/bin/pre_deploy#L29

It looks like we always call load_initial_data (after running the migrations). So I think the appropriate fix is:

the migration just needs to not convert existing branches to RepositoryBranch records and we can let the fixture do its thing.

I'll update this PR and we can try again.

@ahal ahal force-pushed the refactor_branch_model branch from 43aaae6 to 21a1056 Compare May 13, 2026 15:14
Comment thread treeherder/model/models.py Outdated
ahal added 5 commits May 13, 2026 16:03
Each transformer can have its own bespoke logic for resolving a
repository. So instead of having an if/else at the call site, move this
logic onto a helper method of the transformer itself for cleaner code.
…yBranch model

The Repository model has a "branch" field, which means repositories are
inherently tied to single branch. It was likely designed this way
because we colloquially called Mercurial repos "branches", so there was
a 1:1 mapping of branches to repositories. But as Github repos started
getting added, this was obviously no longer the case.

To support Github, we started adding hacks that overrode the "branch"
field. We turned it into a comma separate list that gets parsed by a
regex to support multiple branches. We added the string "pull request"
to support pull requests.

This worked, but it's inherently limiting. For example, pushes that
match the comma separated regex lose the information of which branch
they actually matched. This is valuable information that Treeherder
could be using in its UI and API.

Instead, we can store branches in a separate table in the DB. This
decouple branches from repository in the model and allows for more
sophisticated configurations.

It will also make it easier to implement "wildcard" support for
branches, which will be needed for "try-like" repositories.
As part of the Gecko2Github migration, we need a way for Treeherder to
display _all_ pushes to certain repositories in order to support an
equivalent to "try".
@ahal ahal force-pushed the refactor_branch_model branch from 21a1056 to 9003b67 Compare May 14, 2026 20:18
@ahal
Copy link
Copy Markdown
Member Author

ahal commented May 14, 2026

I updated this PR such that instead of accepts_all_branches, it's possible to use wildcards in branch names. You can use * to match all branches, or releases/* to match only some branches. If used, wildcards must be a suffix of the name.

This will allow us to fix a request from the firefox-ios team.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants