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

respect GAE_ENV=localdev #283

merged 1 commit into from Jul 13, 2022

respect GAE_ENV=localdev #283

merged 1 commit into from Jul 13, 2022


Copy link

@zevdg zevdg commented Jul 12, 2022

The public docs at tell you that GAE_ENV is the value to switch on to determine if you're running in devappserver or not. appengine.IsDevAppserver() should obviously use the same variable that we encourage users to check directly.

@zevdg zevdg requested a review from jinglundong July 12, 2022 17:28
@@ -131,5 +131,5 @@ func fullyQualifiedAppID(_ netcontext.Context) string {

func IsDevAppServer() bool {
return os.Getenv("RUN_WITH_DEVAPPSERVER") != ""
return os.Getenv("RUN_WITH_DEVAPPSERVER") != "" || os.Getenv("GAE_ENV") == "localdev"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

FYI: in the case where these two variable disagree (e.g. RUN_WITH_DEVAPPSERVER=1 GAE_ENV=standard) this code trusts the legacy RUN_WITH_DEVAPPSERVER over the new GAE_ENV. IMO this is the best option as it is less likely to break existing test environments that may be configured this way, and lets people start using the new variable in those envs. We can update this logic and/or totally remove RUN_WITH_DEVAPPSERVER in a later release.

@zevdg zevdg force-pushed the gae_env_localdev branch 2 times, most recently from be85e9f to 7071ee1 Compare July 12, 2022 18:29
@zevdg zevdg merged commit 65ad43f into golang:master Jul 13, 2022
17 checks passed
@zevdg zevdg deleted the gae_env_localdev branch July 13, 2022 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
None yet

Successfully merging this pull request may close these issues.

None yet

2 participants