Details
- Reviewers
hach-que chad - Maniphest Tasks
- T9352: Write some custom Herald glue for driving builds through Owners
- Commits
- Restricted Diffusion Commit
rP2728a9f9649e: Allow builds to have parameters
With a custom Herald rule + action like the one in T9352, updated a revision and saw it generate multiple builds with varying parameters.
Diff Detail
- Repository
- rP Phabricator
- Branch
- bp1
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 8148 Build 9305: Run Core Tests Build 9304: arc lint + arc unit
Event Timeline
I noticed this doesn't enforce uniqueness over build parameters, does this mean multiple requests with the same parameter will result in multiple builds?
Does this change mean that builds with no parameters will also be instantiated twice? (At the moment I believe they're forced to be unique across buildPlanPHID and buildablePHID)
Yeah -- if you have two Herald rules which run the same build plan, you'll now get two builds out of it.
There was no existing constraint that prevented this before, per se, but it was very hard (impossible in the upstream?) to get into that state because:
- Herald implicitly de-duplicated by plan PHID within a single run of the rules.
- Commit rules only run when the commit is pushed.
- Revision rules only run when the revision is updated, which always creates a new buildable.
- Manually running builds also creates a new buildable.
I did trigger multiple copies of a build earlier in T9352, but it required a custom Herald action to do it (P1860).
My assumption is that it will be reasonable to adjust Herald rules to not trigger duplicate builds, but maybe this won't hold up in practice.
I think we're reasonably likely to end up with some kind of deduplication eventually, I'm just not sold on "all parameters are the same" being the best ruleset. I don't think it's a bad ruleset, but it feels like it's a little too implicit/magical to me.
In the case of T9352, for example, I might want to add a "triggering herald rule PHID" or "triggering package PHID" parameter. Those could be pretty useful diagnostically, but would affect the "all parameters the same" deduplication rule. It seems questionable to choose a rule which prevents this kind of thing.