Skip to content
This repository has been archived by the owner on Nov 5, 2019. It is now read-only.

Force GCE metadata querying to be done without proxies #120

Closed
wants to merge 9 commits into from

Conversation

erwanor
Copy link
Contributor

@erwanor erwanor commented Jan 22, 2015

Following issue #114

@erwanor
Copy link
Contributor Author

erwanor commented Jan 24, 2015

I understand that this draft is failing the test because urlopen should be called at least once. (see test_oauth2client.py. That being said according to urllib2.py urlopen ends up making a call to open anyway. Is that ok @craigcitro ?

@craigcitro
Copy link
Contributor

@aaronwinter i suspect it's just that the call doesn't match the expected one in the mock; can you look at updating it? (it's probably just that it needs the args to match.)

@@ -938,7 +938,8 @@ def _detect_gce_environment(urlopen=None):
# the metadata resolution was particularly slow. The latter case is
# "unlikely".
try:
response = urlopen('http://169.254.169.254/', timeout=1)
no_proxy_conf = urllib.request.build_opener(urllib.request.ProxyHandler({}))

This comment was marked as spam.

@erwanor
Copy link
Contributor Author

erwanor commented Jan 28, 2015

@craigcitro I have updated the arguments passed to open but the test is still failing. I am not very well versed with unittest. Can you take a quick look at it?

return_value=response,
autospec=True) as urlopen:
autospec=True) as open:

This comment was marked as spam.

@nathanielmanistaatgoogle
Copy link
Contributor

Please squash commits and push a new draft that passes tests?

Squash commits from master repository
Previously the mock's name was overriding the built-in 'open', also the arguments were no matching correctly for both test_get_environment_gce_production and test_get_environment_unknown. This should fix the problem!
@dhermes
Copy link
Contributor

dhermes commented Aug 31, 2015

@nathanielmanistaatgoogle I'm closing this since #293 handles it. @aaronwinter Feel free to re-open if you feel like my change misses something.

ASIDE: It'd be nice to get it in before cutting 1.5.0.

@dhermes dhermes closed this Aug 31, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants