Also support "preview" features, in addition to "experimental" features #2960
Conversation
This comment has been minimized.
This comment has been minimized.
| @@ -691,13 +691,13 @@ mod tests { | |||
| { | |||
| query: Query | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
I'm not sure why all this whitespace changed within this PR!
| "experimental_logging": "https://github.com/apollographql/router/discussions/1961" | ||
| }, | ||
| "preview": {} |
There was a problem hiding this comment.
Suggest adding a test gated experimental and preview dummy features for testing.
This would mean that tests dont need to change once experimental logging becomes non-experimental and would also allow unit test testing of preview code path.
There was a problem hiding this comment.
Do you mean gated on #[cfg(test)]?
There was a problem hiding this comment.
I think proper tests for this feature would need to be lifecycle tests in order to capture stdout. But those use $CARGO_BIN_EXE_router, the "normal" executable which is compiled without cfg(test).
Do we still want experimental_noop and preview_noop configs for testing, if they are user-visible?
| .collect(); | ||
| if !needed_discussions.is_empty() { | ||
| tracing::info!( | ||
| r#"You're using some "preview" features of the Apollo Router (those which have their configuration prefixed by "preview"). These features are not officially supported with any SLA and may still contain bugs or undergo iteration. You're encouraged to try preview features in test environments to familiarize yourself with upcoming functionality before it reaches general availability. For more information about launch stages, please see the documentation here: https://www.apollographql.com/docs/resources/product-launch-stages/"#, |
There was a problem hiding this comment.
Should this message include which preview features are used, like the corresponding message does for experimental features?
There was a problem hiding this comment.
I'm glad you caught that, too. I was also wondering that, but I cherry picked this off another branch. I meant to either:
- Check it out and try out how it behaved in practice.
- Write a test that snapshotted it.
At this moment, I'm not sure when I'll be able to get back to this PR though, but this point of uncertainty still stands right now.
| let needed_discussions: Vec<String> = used_preview_conf | ||
| .into_iter() | ||
| .filter_map(|used_conf| { | ||
| self.experimental |
There was a problem hiding this comment.
This should be self.preview. I’m fixing this and refactoring to share common code.
This expands on the existing mechanism that was originally introduced in #2242, which supports the notion of an "experimental" feature, and make it compatible with the notion of "preview" features.
SimonSapin
force-pushed
the
abernix/preview_features
branch
from
5e7b136
to
dc26cff
Compare
|
I’ve rebased to fix a merge conflict but kept my commits separate for now. These can be squashed after review. They refactor the code a bit, add tests, and add a changelog entry. Please review! |
SimonSapin
force-pushed
the
abernix/preview_features
branch
from
6940f1a
to
ac193bf
Compare
…es (#2960) This expands on the existing mechanism that was originally introduced in #2242, which supports the notion of an "experimental" feature, and make it compatible with the notion of "preview" features. **Checklist** Complete the checklist (and note appropriate exceptions) before a final PR is raised. - [x] Changes are compatible[^1] - [x] Documentation[^2] completed - [-] Performance impact assessed and acceptable - Tests added and passing[^3] - [-] Unit Tests - [x] Integration Tests - [x] Manual Tests --------- Co-authored-by: o0Ignition0o <jeremy.lempereur@gmail.com> Co-authored-by: Simon Sapin <simon@apollographql.com>
This expands on the existing mechanism that was originally introduced in #2242, which supports the notion of an "experimental" feature, and make it compatible with the notion of "preview" features.
Most of this code was written by @o0Ignition0o, I just extracted it and put it here! 😄
Checklist
Complete the checklist (and note appropriate exceptions) before a final PR is raised.