Add a timeout for GCE detection #101 Introduces too many false negatives

[bmkessler]

PR #101 added a timeout to metadata request in _detect_gce_environment as a temporary workaround for this request taking too long for some people with the comment below:

oauth2client/client.py L1110-1117

NOTE: The explicit timeout is a workaround. The underlying

issue is that resolving an unknown host on some networks will take

20-30 seconds; making this timeout short fixes the issue, but

could lead to false negatives in the event that we are on GCE, but

the metadata resolution was particularly slow. The latter case is

"unlikely".

connection = six.moves.http_client.HTTPConnection(
_GCE_METADATA_HOST, timeout=1)

Note that we do observe many false negatives in production on GCE so that the later case is not "unlikely." Furthermore, it appears that since the time when people were having issues with the original call taking too long, the code was changed in multiple ways such that the long time to resolve an unknown host is not an issue. First, there is now a NO_GCE_CHECK environment variable so that people can avoid the check entirely if they want.

oauth2client/client.py L113
# If set to True _get_environment avoid GCE check (_detect_gce_environment) NO_GCE_CHECK = os.environ.setdefault('NO_GCE_CHECK', 'False')

oauth2client/client.py L1167
if NO_GCE_CHECK != 'True' and _detect_gce_environment():

Second, the GCE credentials are the last in the list of

oauth2client/client.py L1376-1380
environ_checkers = [ cls._implicit_credentials_from_gae, cls._implicit_credentials_from_files, cls._implicit_credentials_from_gce, ]

So that this call would only be invoked if the other attempts to get credentials have already failed, so this would be the last check to fail and hence users would not want to terminate it early.

I propose removing the timeout from the oauth2client/client.py L1117

[nathanielmanistaatgoogle]

@Galabar001 and @jonparrott: having recently considered a separate change in this area, what are your thoughts and reactions here?

[theacodes]

I'd be for raising the timeout to something reasonable (10s) or removing it altogether.

[Galabar001]

I'm working on getting something unambiguous in the DMI information. With
that, we should be able to avoid the check.

However, that would only be for GCE. Unfortunately, because it would be
difficult to update old instances, we would need to keep the connection
check in for a while (causing non-GCE clients to continue feeling the pain
of that check).
On Jul 11, 2016 9:40 AM, "Jon Wayne Parrott" [email protected]
wrote:

I'd be for raising the timeout to something reasonable (10s) or removing
it altogether.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#538 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AKx3H6rvpd-Y5okk5B23AAbf27HjePqrks5qUnIAgaJpZM4JFmG6
.

[theacodes]

Since we're about to make a breaking release (3.0.0), does anyone wanna take on raising the timeout in time?