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

contrib/_appengine_environ.py KeyError SERVER_SOFTWARE #1470

Closed
wittfabian opened this issue Nov 2, 2018 · 5 comments · Fixed by #1704
Closed

contrib/_appengine_environ.py KeyError SERVER_SOFTWARE #1470

wittfabian opened this issue Nov 2, 2018 · 5 comments · Fixed by #1704

Comments

@wittfabian
Copy link

wittfabian commented Nov 2, 2018

https://github.com/urllib3/urllib3/blob/master/src/urllib3/contrib/_appengine_environ.py

In the development environment, the variable SERVER_SOFTWARE is sometimes not set, so the query should read as follows:

os.environ.get('SERVER_SOFTWARE', '').startswith('Development')

def is_local_appengine():
    return ('APPENGINE_RUNTIME' in os.environ and
            os.environ.get('SERVER_SOFTWARE', '').startswith('Development'))


def is_prod_appengine():
    return ('APPENGINE_RUNTIME' in os.environ and
            not os.environ.get('SERVER_SOFTWARE', '').startswith('Development') and
            not is_prod_appengine_mvms())
@sethmlarson
Copy link
Member

I would accept a PR that makes this change. cc: @theacodes

@theacodes
Copy link
Member

Yeah, fine with me. I'd be curious to know when the local server doesn't set the SERVER_SOFTWARE and what effect that has on detection.

@wittfabian
Copy link
Author

The Travis CI build could not complete ...

@wittfabian
Copy link
Author

Any innovations here? @pquentin @theacodes

@zevdg
Copy link
Contributor

zevdg commented Oct 3, 2019

I just ran into this bug, (simplified reproduction of how I ran into it https://repl.it/repls/ThinSpatialNumerator) and was about to submit a fix much like the two that have already been proposed.

There is some new, and possibly relevant information. This function

def is_prod_appengine_mvms():
    return os.environ.get('GAE_VM', False) == 'true'

seems to always returns False now.

App Engine stopped setting this variable and removed it from their PHP docs (I couldn't find any references to it in the python docs).

It looks like the reasons the original CL never made it in seem to be because they didn't work quite right on flex, but since the flex check isn't working anymore anyway, and no one seems to be complaining about it, I suggest we move forward with the fix now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants