Page MenuHomePhabricator

Prevent 'Start or Wait for Build Plan' from running disabled build plans
AbandonedPublic

Authored by hach-que on Jul 16 2015, 5:39 AM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Apr 25, 12:10 AM
Unknown Object (File)
Fri, Apr 19, 2:22 AM
Unknown Object (File)
Fri, Apr 19, 2:22 AM
Unknown Object (File)
Thu, Apr 11, 8:27 AM
Unknown Object (File)
Thu, Apr 11, 3:41 AM
Unknown Object (File)
Fri, Mar 29, 12:13 AM
Unknown Object (File)
Mar 16 2024, 7:51 PM
Unknown Object (File)
Mar 5 2024, 7:58 AM
Subscribers

Details

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

Ref T1049. This prevents the 'Start or Wait for Build Plan' step from running a disabled build plan. If the target build plan is disabled, the build step returns successfully and logs to indicate that the build was skipped.

Test Plan

Tested on local development environment.

Diff Detail

Branch
master
Lint
Lint Passed
Unit
Test Failures
Build Status
Buildable 7289
Build 7617: [Placeholder Plan] Wait for 30 Seconds
Build 7616: arc lint + arc unit

Event Timeline

hach-que retitled this revision from to Prevent 'Start or Wait for Build Plan' from running disabled build plans.
hach-que updated this object.
hach-que edited the test plan for this revision. (Show Details)
hach-que added a reviewer: epriestley.
epriestley edited edge metadata.

Why is this passing instead of failing?

This revision now requires changes to proceed.Jul 16 2015, 8:41 PM

If it were failing, you'd need to go and remove all the build steps that call the disabled build plan. The scenario that this issue was raised in is that a team wanted to temporarily not deploy to one datacenter, so they disabled the build plan for that datacenter.

Basically, if this failed build steps for disabled build plans, then you might as well just remove the build step instead of disabling the plan.

(to be honest I'd like to have maybe a "skipped" build target status so it's obvious nothing happened, but that feels like a larger change than this bug fix)

I think this scenario would be better handled with a "Skip" or "Pass" build step than treating disabled plans as automatic passes. That behavior seems very unintuitive to me.

You mean you'd rather have a build step you add to a build plan that skips the rest of the build targets?

I was also thinking we could allow build steps to be disabled from the UI, so people could just disable individual steps.

Presumably this will be addressed when upstream implements running or embedding build plans.