Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Feature flags #368

Closed
jromero opened this issue Oct 24, 2019 · 5 comments · Fixed by #576
Closed

Proposal: Feature flags #368

jromero opened this issue Oct 24, 2019 · 5 comments · Fixed by #576
Labels
status/discussion-needed Issue or PR that requires in-depth discussion.

Comments

@jromero
Copy link
Member

jromero commented Oct 24, 2019

As we strive for a time-based release cadence instead of feature-based releases there are times where it doesn't make sense to have a partially implemented feature (eg. packages #353) or features that aren't yet intended for GA. Although these feature are not "ready" certain individual might want to start experimenting with said features. Here are a few proposals to standardize the way in which we provide additional incomplete or experimental features.

Build Time Enablement

Option 1 - go build ldflags

Setting variables at build time could allow us to completely gate code/features that are incomplete.

go build -ldflags "-X 'github.com/buildpack/pack/feature.FeatureA=true'" ...

Runtime Enablement

Option 1 - ~/.pack/config.toml individual features

A list of experimental features in the configuration.

Example:

# Enable experimental features (false by default)
[experimental]
packages = true
slices = false

Option 2 - ~/.pack/config.toml single configuration

A single configuration element enables all experimental features.

Example:

# Enable experimental features (false by default)
experimental = true
@jromero jromero added the status/discussion-needed Issue or PR that requires in-depth discussion. label Oct 24, 2019
@simonjjones
Copy link
Member

The real question we should answer is whether we want build time or runtime feature flagging, they could each be implemented in a number of ways. I think the answer is possibly both, they solve different problems.

While we are still working on features or groups of features in the main codebase I think it makes sense to continue to ship binaries that have no way of consuming these features. This can be achieved by a build time feature flag that simply doesn't wire up the feature(s) in question. Anyone who builds the project for themselves would be able to use these features, and it's more likely that they'd understand any risks associated with doing so.

Runtime feature flagging makes sense in the context of shipping experimental features; ones where we have a reasonable amount of confidence, but perhaps aren't yet feature complete or we're not ready to call them GA.

@jromero
Copy link
Member Author

jromero commented Oct 25, 2019

@simonjjones Completely agree with most, if not all, points in you comment. I think the main purpose of this issue should be to define the standard mechanisms to do so instead of implementing something last minute and before we know it we have X number of ways to do something similar.

With that said, you bring up a good point about probably needing both build time and runtime enablement. I don't know of many more options for build time enablement but could think of at least another variant to runtime enablement. Instead of individually calling out each individual feature and potentially creating a stale list of experimental features we could set a single flag that enables all experimental features. Similar to the docker experiment config setting.

I'll update my initial comment to break the options into categories and add the single config setting to rule them all.

@jromero jromero added the status/triage Issue or PR that requires contributor attention. label Feb 5, 2020
@natalieparellano natalieparellano removed the status/triage Issue or PR that requires contributor attention. label Feb 5, 2020
@ameyer-pivotal
Copy link
Contributor

ameyer-pivotal commented Apr 13, 2020

The runtime/experimental feature flagging could likely use the command hiding ability of Cobra, like we've been.

I like the idea of build-time feature flagging as well, and the specific mechanism proposed. Something to consider here is the concept of "mainlining" the feature flags (essentially removing them and any gates around the relevant code), so that we don't end up with an ever-growing list of flags. The ultimate solution may want to define the process and timeframe for which flags can be mainlined (the answer is likely simple: mainline either just before or just after GA'ing the feature).

@jromero
Copy link
Member Author

jromero commented Apr 13, 2020

For that sake of keeping things simple, I think we should start with what we need and build out any enhancements to it as we see the need arise. I know @zmackie is looking to add a flag he'd like to enable as experimental. For that case, I vote for runtime enablement option 2. I foresee a lot of maintenance work (as you've mentioned @ameyer-pivotal ) in option 1 without any value at this point in time.

@zmackie
Copy link
Contributor

zmackie commented Apr 14, 2020

Totally agree that option 2 is most useful. I'm working on something right now that will likel be released under an experimental flag. My assumption is that releasing it would entail cleaning up flags.

zmackie added a commit to zmackie/pack that referenced this issue Apr 14, 2020
 - Addresses buildpacks#368

Signed-off-by: Zander Mackie <[email protected]>
zmackie added a commit to zmackie/pack that referenced this issue Apr 16, 2020
 - Addresses buildpacks#368

Signed-off-by: Zander Mackie <[email protected]>
zmackie added a commit to zmackie/pack that referenced this issue Apr 16, 2020
 - Addresses buildpacks#368

Signed-off-by: Zander Mackie <[email protected]>
@jromero jromero linked a pull request Apr 21, 2020 that will close this issue
zmackie added a commit to zmackie/pack that referenced this issue Apr 29, 2020
 - Addresses buildpacks#368

Signed-off-by: Zander Mackie <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/discussion-needed Issue or PR that requires in-depth discussion.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants