Page MenuHomePhabricator

[harbormaster/core] Implement parameterized build plans
AbandonedPublic

Authored by hach-que on Jul 15 2015, 12:09 AM.
Tags
None
Referenced Files
F13123173: D13635.diff
Tue, Apr 30, 8:53 AM
Unknown Object (File)
Sun, Apr 28, 6:08 AM
Unknown Object (File)
Wed, Apr 24, 10:05 PM
Unknown Object (File)
Sun, Apr 21, 4:11 PM
Unknown Object (File)
Fri, Apr 19, 2:22 AM
Unknown Object (File)
Fri, Apr 19, 2:22 AM
Unknown Object (File)
Fri, Apr 19, 2:22 AM
Unknown Object (File)
Fri, Apr 19, 2:22 AM

Details

Reviewers
epriestley
Group Reviewers
Blessed Reviewers
Maniphest Tasks
T1049: Implement Harbormaster
Summary

Ref T1049. This implements support for parameterized build plans, and it works correctly with the "Run Build Plan" build step so that things are restarted correctly when you have a hierarchy of build plans.

Note that you can only pass in parameters via "Run Build Plan"; there's no support for parameters via Herald or anything of that nature.

Test Plan

Tested it locally, about to test it in production.

Diff Detail

Branch
master
Lint
Lint Passed
Unit
Test Failures
Build Status
Buildable 7279
Build 7597: [Placeholder Plan] Wait for 30 Seconds
Build 7596: arc lint + arc unit

Event Timeline

hach-que retitled this revision from to [harbormaster/core] Implement parameterized build plans.
hach-que updated this object.
hach-que edited the test plan for this revision. (Show Details)
hach-que added a reviewer: epriestley.
resources/sql/autopatches/20150715.harbor.1.buildparam.sql
7

This is the equivalent of md5(print_r(array(), true)); since existing builds have no parameters.

hach-que edited edge metadata.

Fix viewing builds where build parameters is null

Allow variables to be placed inside build parameters

Merge parameters into build and target names

Return array() if build parameters are null

Allow capital letters in Harbormaster variables

epriestley edited reviewers, added: hach-que; removed: epriestley.

I think we're ready to upstream some flavor of this for T9352, although I think this particular diff is on top of some stuff like D9807 or some other sub-build stuff that I don't want to upstream yet, so I'm just going to steal it and separate the components. I think things will otherwise look mostly similar. One thing I'm probably going to do is put these parameters in their own namespace, like ${build/x.y.z}?

Putting a text box on "Run Build Plan" makes perfect sense and I hadn't thought of it (T9352 is only coming at this from the Herald side). We could provide a flag for bin/harbormaster build too.

You should just be able to grab this and reset the src/applications/harbormaster/step/HarbormasterRunBuildPlanBuildImplementation.php to make it non-dependant on D9807. That said, I don't know how people would use this without the "Run Build Plan" step, since that's kind of what makes it useful.

The parameter bit of this landed in D14222. Some parts that didn't:

  • Setting parameters from the manual build dialog -- totally reasonable, just not much of an upstream use case yet. CLI would be similarly useful.
  • Putting a tab on builds to show the parameters? You can see them on build targets but not the build itself.
  • The deduplication stuff, some discussion in D14222.
  • Parameterized names -- I think this is actually pretty cool, but maybe better as a separate field? Then we don't have to show the un-merged name anywhere.
hach-que abandoned this revision.
hach-que edited reviewers, added: epriestley; removed: hach-que.

Upstream has a different implementation of this now which is sort of partially this diff. Abandoning it because any remaining features would need to be rewritten on top of the new stuff anyway.