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

Move persisted queries from preview to general availability #3914

Merged
merged 1 commit into from
Sep 28, 2023

Conversation

glasser
Copy link
Member

@glasser glasser commented Sep 27, 2023

Persisted Queries
(https://www.apollographql.com/docs/graphos/operations/persisted-queries/), a GraphOS Enterprise feature, is now moving to General Availability, from Preview where it has been since Apollo Router 1.25.

For more information about launch stages, please see the documentation here:
https://www.apollographql.com/docs/resources/product-launch-stages/

The feature is now configured with a persisted_queries top-level key in the YAML configuration instead of with preview_persisted_queries. Existing configuration files will keep working as before, only with a warning. To fix that warning, rename the configuration section like so:

-preview_persisted_queries:
+persisted_queries:
   enabled: true

@glasser glasser requested a review from a team as a code owner September 27, 2023 02:12
@github-actions

This comment has been minimized.

@router-perf
Copy link

router-perf bot commented Sep 27, 2023

CI performance tests

  • events_big_cap_high_rate - Stress test for events with a lot of users, deduplication enabled and high rate event with a big queue capacity
  • events_without_dedup - Stress test for events with a lot of users and deduplication DISABLED
  • events - Stress test for events with a lot of users and deduplication ENABLED
  • large-request - Stress test with a 1 MB request payload
  • step - Basic stress test that steps up the number of users over time
  • xlarge-request - Stress test with 10 MB request payload
  • reload - Reload test over a long period of time at a constant rate of users
  • no-graphos - Basic stress test, no GraphOS.
  • xxlarge-request - Stress test with 100 MB request payload
  • step-jemalloc-tuning - Clone of the basic stress test for jemalloc tuning
  • const - Basic stress test that runs with a constant number of users

Persisted Queries
(https://www.apollographql.com/docs/graphos/operations/persisted-queries/),
a GraphOS Enterprise feature, is now moving to General Availability,
from Preview where it has been since Apollo Router 1.25.

For more information about launch stages, please see the documentation
here:
https://www.apollographql.com/docs/resources/product-launch-stages/

The feature is now configured with a `persisted_queries` top-level key
in the YAML configuration instead of with `preview_persisted_queries`.
Existing configuration files will keep working as before, only with a
warning. To fix that warning, rename the configuration section like so:

```diff
-preview_persisted_queries:
+persisted_queries:
   enabled: true
```
@glasser glasser force-pushed the glasser/persisted-queries-ga branch from 415245d to f7a901d Compare September 27, 2023 02:15
@glasser
Copy link
Member Author

glasser commented Sep 27, 2023

This PR was heavily inspired by #3356.
Note that it does not touch feature_discussions.json, because (mistakenly?) this feature was never added to it.

@@ -206,7 +206,7 @@ impl Metrics {
);
log_usage_metrics!(
value.apollo.router.config.persisted_queries,
"$.preview_persisted_queries[?(@.enabled == true)]",
"$.persisted_queries[?(@.enabled == true)]",
Copy link
Member Author

Choose a reason for hiding this comment

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

@BrynCooke I just want to double-check my understanding that this expression is only ever applied to a YAML file that has been "migrated".

Copy link
Contributor

Choose a reason for hiding this comment

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

That's correct.

@@ -0,0 +1,5 @@
description: Persisted queries are no longer preview, `preview_persisted_queries` is renamed `persisted_queries`
Copy link
Member Author

Choose a reason for hiding this comment

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

Text chosen for consistency with 0008, though I would probably use a semicolon and add "to" after "renamed"

@@ -178,7 +178,7 @@ impl LicenseEnforcementReport {
.name("Operation aliases limiting")
.build(),
ConfigurationRestriction::builder()
.path("$.preview_persisted_queries")
.path("$.persisted_queries")
Copy link
Member Author

Choose a reason for hiding this comment

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

Similarly want to double-check my understanding that this expression is only ever applied to a YAML file that has been "migrated".

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, both license enforcement and metrics are applied to Configuration::validated_yaml which has already had migrations applied.

@@ -6,8 +6,6 @@ minVersion: 1.25.0

<GraphOSEnterpriseRequired />

<PreviewFeature discordLink="https://discordapp.com/channels/1022972389463687228/1143901714173407342"/>
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that there are other references to "preview" on this page, but they are loaded via components in the docs repo.

@glasser glasser requested review from a team, SimonSapin, BrynCooke and bnjjj September 27, 2023 02:28
Copy link
Contributor

@Meschreiber Meschreiber left a comment

Choose a reason for hiding this comment

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

Thanks for this @glasser ! Will keep an eye out here for the final configuration name and update apollographql/docs#565 accordingly

@bnjjj bnjjj merged commit 0d1c044 into dev Sep 28, 2023
1 of 2 checks passed
@bnjjj bnjjj deleted the glasser/persisted-queries-ga branch September 28, 2023 14:12
@Geal Geal mentioned this pull request Oct 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants