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

xds: cleanup old env vars #5888

Open
easwars opened this issue Dec 22, 2022 · 2 comments
Open

xds: cleanup old env vars #5888

easwars opened this issue Dec 22, 2022 · 2 comments
Labels
Area: xDS Includes everything xDS related, including LB policies used with xDS. fixit P2 Type: Internal Cleanup Refactors, etc

Comments

@easwars
Copy link
Contributor

easwars commented Dec 22, 2022

We introduce environment variables when we add new features. Initially the default value for these env vars is set to false and the user will have to explicitly set the value to true to enable the feature.

At some point, when we think the feature is stable enough, we set the default value to true. At this point, the user will have to explicitly set the value to false to disable the feature.

One release after setting the default value to true, we should delete the environment variable, which will get rid of conditional code and eventually lead to simpler code. Even though, we have had this policy for a while, env vars have been lingering forever. We should do a one time cleanup for now, and going forward, we should file issues and link them in TODOs for cleanup for future env vars.

@arvindbr8 arvindbr8 added the P2 label Jan 3, 2023
@purnesh42H purnesh42H added the Area: xDS Includes everything xDS related, including LB policies used with xDS. label Sep 7, 2024
@easwars
Copy link
Contributor Author

easwars commented Sep 26, 2024

As part of #6749, we did end up removing a bunch of env vars.

But some still remain:

  1. LeastRequestLB:
    LeastRequestLB = boolFromEnv("GRPC_EXPERIMENTAL_ENABLE_LEAST_REQUEST", false)
    This has been defaulting to false from the time it was added (about a year ago). We should set the default to true now and get rid of it in the next release.
  2. EnforceALPNEnabled:
    Was defaulted to true in the last release. So, maybe as part of the next release, we can delete it.

pnikonowicz added a commit to pnikonowicz/grpc-go that referenced this issue Dec 10, 2024
part of issue grpc#5888

removes the ability to toggle ALPN
enforcment as the default is true
and there is a development policy
in place that when env values
become true by default, the toggle
behavior should be removed
@arjan-bal
Copy link
Contributor

We need to provide a mechanism for older users to disable ALPN enforcement before we remove the env variable for ALPN: #7769

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: xDS Includes everything xDS related, including LB policies used with xDS. fixit P2 Type: Internal Cleanup Refactors, etc
Projects
None yet
Development

No branches or pull requests

4 participants