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

Experiments supported only in alpha/dev builds #30948

Merged
merged 1 commit into from
Jun 17, 2022

Conversation

apparentlymart
Copy link
Contributor

We originally introduced the idea of language experiments as a way to get early feedback on not-yet-proven feature ideas, ideally as part of the initial exploration of the solution space rather than only after a solution has become relatively clear.

Unfortunately, our tradeoff of making them available in normal releases behind an explicit opt-in in order to make it easier to participate in the feedback process had the unintended side-effect of making it feel okay to use experiments in production and endure the warnings they generate. This in turn has made us reluctant to make use of the experiments feature lest experiments become de-facto production features which we then feel compelled to preserve even though we aren't yet ready to graduate them to stable features.

In an attempt to tweak that compromise, here we make the availability of experiments at all a build-time flag which will not be set by default, and therefore experiments will not be available in most release builds.

The intent (not yet implemented in this PR) is for our release process to set this flag only when it knows it's building an alpha release or a development snapshot not destined for release at all, which will therefore allow us to still use the alpha releases as a vehicle for giving feedback participants access to a feature (without needing to install a Go toolchain) but will not encourage pretending that these features are production-ready before they graduate from experimental.

Only language experiments have an explicit framework for dealing with them which outlives any particular experiment, so most of the changes here are to that generalized mechanism. However, the intent is that non-language experiments, such as experimental CLI commands, would also in future check the Meta.AllowExperimentalFeatures flag added here and gate the use of those experiments too, so that we can be consistent that experimental features will never be available unless you explicitly choose to use an alpha release or a custom build from source code.


Since there are already some experiments active at the time of this commit which were not previously subject to this restriction, we'll pragmatically leave those as exceptions that will remain generally available for now, and so this new approach will apply only to new experiments started in the future. Once those experiments have all concluded, we will be left with no more exceptions unless we explicitly choose to make an exception for some reason we've not imagined yet.


It's important that we be able to write tests that rely on experiments either being available or not being available, so here we're using our typical approach of making "package main" deal with the global setting that applies to Terraform CLI executables while making the layers below all support fine-grain selection of this behavior so that tests with different needs can run concurrently without trampling on one another.

As a compromise, the integration tests in the terraform package will run with experiments enabled by default since we commonly need to exercise experiments in those tests, but they can selectively opt-out if they need to by overriding the loader setting back to false again.

@apparentlymart
Copy link
Contributor Author

I'm currently working on something that seems likely to become an experiment, at least for a little while. Given that this includes carveouts for the two existing experiments where they will still remain available, and that we'll remain able to tweak this logic as needed as we get more experience using it, I'd like to move ahead with this on a meta-experimental basis just so that the "experiments allowed" flag will be available for my new code to refer to and thus get some experience with this approach.

Our current release process doesn't understand how to add the extra linker flag yet, even when producing alpha builds, so for the moment any feature guarded by this will not be available in any release builds and will require a custom build directly from source to use. I am planning to continue work on adopting the new release process later in this development period, and the new process does know how to set this flag appropriately and so the new stuff I'm planning to add guarded by this flag will become available from the first alpha release we produce using the new release process, once that's ready to go.

@apparentlymart apparentlymart requested a review from a team June 16, 2022 21:22
Copy link
Contributor

@alisdair alisdair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me! Two comments:

  1. Since we are just about to mark the module_variable_optional_attrs experiment as concluded in Edit type constraints docs for style and flow #31210, I don't think we need the part of the diff here which special-cases it.

  2. Given the narrow scope in which experiments will now be available, do you think we can remove the warning when an experiment is enabled? It seems redundant to me, and removing it would simplify the local go install command for working with an experimental feature, since there would be no need to also override the disableExperimentWarnings flag.

@apparentlymart
Copy link
Contributor Author

Ah yes, I suppose you are about to remove the code I modified for handling the module_variable_optional_attrs experiment here and so this PR will become conflicted at that point. I'll drop that so that neither of us will need to deal with the conflict (depending on who ends up going first here).

Your second point about removing the warning message does seem well-reasoned and I think I agree with you that knowing you're using an alpha release is enough to know that anything that release might do is subject to change. I'd like to save that for a subsequent PR just because I'd like to land this in the form I already tested so I can use it in another branch I'm working on, but I'll make a note to revisit that in the near future. Thanks!

@apparentlymart
Copy link
Contributor Author

apparentlymart commented Jun 17, 2022

I'm going to hold on merging this for the moment because I'm currently focused on something else, but I'll aim to get this landed soon, with the module_variable_optional_attrs carveout removed. @alisdair, please don't block on me for moving forward with the stabilization of optional attributes in the main branch.

We originally introduced the idea of language experiments as a way to get
early feedback on not-yet-proven feature ideas, ideally as part of the
initial exploration of the solution space rather than only after a
solution has become relatively clear.

Unfortunately, our tradeoff of making them available in normal releases
behind an explicit opt-in in order to make it easier to participate in the
feedback process had the unintended side-effect of making it feel okay
to use experiments in production and endure the warnings they generate.
This in turn has made us reluctant to make use of the experiments feature
lest experiments become de-facto production features which we then feel
compelled to preserve even though we aren't yet ready to graduate them
to stable features.

In an attempt to tweak that compromise, here we make the availability of
experiments _at all_ a build-time flag which will not be set by default,
and therefore experiments will not be available in most release builds.

The intent (not yet implemented in this PR) is for our release process to
set this flag only when it knows it's building an alpha release or a
development snapshot not destined for release at all, which will therefore
allow us to still use the alpha releases as a vehicle for giving feedback
participants access to a feature (without needing to install a Go
toolchain) but will not encourage pretending that these features are
production-ready before they graduate from experimental.

Only language experiments have an explicit framework for dealing with them
which outlives any particular experiment, so most of the changes here are
to that generalized mechanism. However, the intent is that non-language
experiments, such as experimental CLI commands, would also in future
check Meta.AllowExperimentalFeatures and gate the use of those experiments
too, so that we can be consistent that experimental features will never
be available unless you explicitly choose to use an alpha release or
a custom build from source code.

Since there are already some experiments active at the time of this commit
which were not previously subject to this restriction, we'll pragmatically
leave those as exceptions that will remain generally available for now,
and so this new approach will apply only to new experiments started in the
future. Once those experiments have all concluded, we will be left with
no more exceptions unless we explicitly choose to make an exception for
some reason we've not imagined yet.

It's important that we be able to write tests that rely on experiments
either being available or not being available, so here we're using our
typical approach of making "package main" deal with the global setting
that applies to Terraform CLI executables while making the layers below
all support fine-grain selection of this behavior so that tests with
different needs can run concurrently without trampling on one another.

As a compromise, the integration tests in the terraform package will
run with experiments enabled _by default_ since we commonly need to
exercise experiments in those tests, but they can selectively opt-out
if they need to by overriding the loader setting back to false again.
@github-actions
Copy link

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants