Page MenuHomePhabricator
Differential D13635

[harbormaster/core] Implement parameterized build plans
AbandonedPublic
Actions

Authored by hach-que on Jul 15 2015, 12:09 AM.
Tags
None
Referenced Files
F14369523: D13635.diff
Fri, Dec 20, 4:55 PM
Unknown Object (File)
Wed, Dec 18, 6:15 AM
Unknown Object (File)
Sat, Dec 14, 5:52 PM
Unknown Object (File)
Fri, Dec 13, 2:54 PM
Unknown Object (File)
Fri, Dec 13, 2:01 AM
Unknown Object (File)
Thu, Dec 12, 7:37 AM
Unknown Object (File)
Wed, Dec 11, 11:04 AM
Unknown Object (File)
Mon, Dec 9, 3:35 PM
View All Files
Subscribers
epriestley
Korvin
• pasik

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

Event Timeline

hach-que updated this revision to Diff 32948.Jul 15 2015, 12:09 AM
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.
hach-que added a task: T1049: Implement Harbormaster.
Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptJul 15 2015, 12:09 AM
Herald added subscribers: Korvin, epriestley. · View Herald Transcript
hach-que added inline comments.Jul 15 2015, 12:10 AM
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 updated this revision to Diff 32949.Jul 15 2015, 12:29 AM
hach-que edited edge metadata.

Fix viewing builds where build parameters is null

hach-que updated this revision to Diff 32950.Jul 15 2015, 1:12 AM

Allow variables to be placed inside build parameters

hach-que updated this revision to Diff 32951.Jul 15 2015, 5:14 AM

Merge parameters into build and target names

hach-que updated this revision to Diff 32954.Jul 15 2015, 6:31 AM

Return array() if build parameters are null

hach-que updated this revision to Diff 32955.Jul 15 2015, 6:55 AM

Allow capital letters in Harbormaster variables

hach-que mentioned this in D9807: [harbormaster/run-build-plan] Implement build step for running another build plan.Aug 8 2015, 10:49 PM
hach-que updated this revision to Diff 33794.Aug 24 2015, 3:05 PM

Rebase

epriestley commandeered this revision.Oct 1 2015, 2:51 PM
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.

epriestley mentioned this in T9352: Write some custom Herald glue for driving builds through Owners.Oct 1 2015, 3:05 PM
hach-que edited edge metadata.Oct 2 2015, 12:46 AM

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.

epriestley mentioned this in D14222: Allow builds to have parameters.Oct 2 2015, 12:31 PM
epriestley mentioned this in rP2728a9f9649e: Allow builds to have parameters.Oct 2 2015, 1:32 PM
epriestley added a comment.Oct 2 2015, 1:32 PM

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 commandeered this revision.Dec 3 2015, 8:55 AM
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.

hach-que mentioned this in T10870: Allow Harbormaster builds to run another build plan on the same buildable.Apr 25 2016, 11:11 PM
• pasik added a subscriber: • pasik.Oct 16 2017, 7:26 PM

Revision Contents
Changeset List

Diff 33794

View Options

resources/sql/autopatches/20150715.harbor.1.buildparam.sql

Loading...
View Options

src/applications/harbormaster/controller/HarbormasterBuildViewController.php

Loading...
View Options

src/applications/harbormaster/controller/HarbormasterBuildableViewController.php

Loading...
View Options

src/applications/harbormaster/step/HarbormasterRunBuildPlanBuildImplementation.php

Loading...
View Options

src/applications/harbormaster/storage/HarbormasterBuildable.php

Loading...
View Options

src/applications/harbormaster/storage/build/HarbormasterBuild.php

Loading...
View Options

src/applications/harbormaster/storage/build/HarbormasterBuildTarget.php

Loading...
Phabricator · Built by Phacility