ADR: Feature Gating Strategy #178
Conversation
|
Do we want any discussion around high level implementation here, at least as far as the surface area exposed directly to the user? For example, consideration of where we may expose this to the user (a Or, is the goal just to get high level agreement that we want feature gates, and any discussions around what that would practically look like and how we'd expose it to the user are deferred? |
|
I think the main point of the PR is to get agreement around using the OpenShift terminology of |
|
That's right, this is about the specific, forward-looking strategy for feature gating. The specific environment variable can be argued over in code review. Good question regarding the scope of applicability - this is why ADRs are generally supposed to go into the codebase they affect, as I have been advocating for. Since this is in the dev-docs repository, I suppose it implicitly would go for any codebase that makes it into the final product. |
There was a problem hiding this comment.
I see the value here and generally agree with what is outlined in the doc. I would like to maybe see something about how we communicate various levels of "support" for features to users - for example, we could have sections in the CHANGELOG.md for this, e.g. RAG could be put under a Dev Preview section in the CHANGELOG.md for the 0.23.0 release of Core
|
Agreed @nathan-weinberg - added that to the |
There was a problem hiding this comment.
Maybe I'm missing something here from this document, but I'm struggling to understand why OpenShift's feature-gating process is relevant to InstructLab?
In general I imagine that any product consuming InstructLab should be responsible for its own implementation of a gating system. I'm not sure why the responsibility of this would come down to an upstream project.
I'm not against a system that proposes a methodology for declaring certain features as experimental, but I feel like doing anything more might be adding more complexity than is needed.
For instance, if a feature is there then it's there, if it's not then it's not. But all of the components present within a codebase must already be in agreement with each other else they wouldn't be there. So I'm not sure why we would want to gate functionality for an upstream project.
|
@RobotSail as described in the ADR, the motivation was to use pre-existing standard terminology within the larger ecosystem, and simply taking inspiration from another project to deal with feature sets before fine-grained per-feature toggles, which we don't need yet (since only one is planned right now, so it's the same level of effort). The reason for feature gating is the same as in any application or product - let users opt into new experimental functionality that might introduce unforeseen regressions or bugs simply by being enabled. In a perfect world, such a thing would never happen and everything would be perfectly modularized, architected, etc., but we do not live in such a world. It's the same concept as alpha functionality. Again as discussed in the doc, a chief goal is to set user expectations regarding experimental features and upgradability.
I don't see what more than that is being proposed. |
|
Hey @anastasds thank you for the detailed response. I'll start off by saying that I fully support this idea. In general, I have no issue with having functionality for declaring features as experimental in an upstream community. As a matter of fact, here's a great community project which provides exactly what you're describing here that we could use as inspiration: https://github.com/ipfs/kubo/blob/master/docs/experimental-features.md Re-reading your ADR, I'm realizing that this is not focused around a product and just using OpenShift as a reference. |
Related ADR: instructlab/dev-docs#178 Depends on #2886 , #2903 Resolves #2888 * Adds feature gating following the pattern in the ADR linked above * Adds an `@dev_preview` decorator that mocks the feature scope environment variable for specific tests covering experimental functionality. * Adds unit tests verifying that RAG related functionality gives a useful and accurate error message when attempted to be used without being enabled. Approved-by: nathan-weinberg Approved-by: cdoern
|
@anastasds if you can squash commits here I'm happy to merge it in, pending no objections from @instructlab/oversight-committee |
Signed-off-by: Anastas Stoyanovsky <[email protected]>
|
@nathan-weinberg done |
No description provided.