Catch an edge case in imports. (issue 7454047)

[jcgre...@google.com]

Reviewers: jcgregorio,

Description:
Catch an edge case in imports.

Please review this at https://codereview.appspot.com/7454047/

Affected files:
M Makefile
M python2/httplib2/__init__.py
A python2/httplib2test_appengine_aberrant.py


Index: Makefile
===================================================================
--- a/Makefile
+++ b/Makefile
@@ -3,8 +3,9 @@
-cd python2 && python2.5 httplib2test.py
-cd python2 && python2.6 httplib2test.py
cd python2 && python2.6 httplib2test_appengine.py
+ cd python2 && python2.6 httplib2test_appengine_aberrant.py
cd python2 && python2.7 httplib2test.py
- cd python2 && python2.7 httplib2test_appengine.py
+ cd python2 && python2.7 httplib2test_appengine_aberrant.py
cd python3 && python3.2 httplib2test.py

VERSION = $(shell python setup.py --version)
Index: python2/httplib2/__init__.py
===================================================================
--- a/python2/httplib2/__init__.py
+++ b/python2/httplib2/__init__.py
@@ -1072,7 +1072,7 @@
raise ImportError # Bail out; we're not actually running on
App Engine.
from google.appengine.api.urlfetch import fetch
from google.appengine.api.urlfetch import InvalidURLError
- except ImportError:
+ except (ImportError, AttributeError):
from google3.apphosting.api import apiproxy_stub_map
if apiproxy_stub_map.apiproxy.GetStub('urlfetch') is None:
raise ImportError # Bail out; we're not actually running on
App Engine.
@@ -1118,7 +1118,7 @@
'http': AppEngineHttpConnection,
'https': AppEngineHttpsConnection
}
-except ImportError:
+except (ImportError, AttributeError):
pass


Index: python2/httplib2test_appengine_aberrant.py
===================================================================
new file mode 100644
--- /dev/null
+++ b/python2/httplib2test_appengine_aberrant.py
@@ -0,0 +1,40 @@
+"""
+httplib2test_appengine
+
+A set of unit tests for httplib2.py on Google App Engine
+
+"""
+
+__author__ = "Joe Gregorio (j...@bitworking.org)"
+__copyright__ = "Copyright 2013, Joe Gregorio"
+
+import os
+import sys
+import unittest
+
+APP_ENGINE_PATH='../../google_appengine'
+
+sys.path.insert(0, APP_ENGINE_PATH)
+
+import dev_appserver
+dev_appserver.fix_sys_path()
+
+from google.appengine.ext import testbed
+testbed = testbed.Testbed()
+testbed.activate()
+testbed.init_urlfetch_stub()
+
+import google.appengine.api
+
+# Force apiproxy_stub_map to None to trigger the test condition to be
tested.
+google.appengine.api.apiproxy_stub_map = None
+
+import httplib2
+
+class AppEngineHttpTest(unittest.TestCase):
+ def test(self):
+ h = httplib2.Http()
+ self.assertFalse(hasattr(httplib2, 'AppEngineHttpsConnection'))
+
+if __name__ == '__main__':
+ unittest.main()

[jcgre...@google.com]

https://codereview.appspot.com/7454047/

[dhe...@google.com]

https://codereview.appspot.com/7454047/diff/4001/Makefile
File Makefile (right):

https://codereview.appspot.com/7454047/diff/4001/Makefile#newcode8
Makefile:8: cd python2 && python2.7 httplib2test_appengine_aberrant.py
Still need to run
python2.7 httplib2test_appengine.py

Also, kill the tab char (red arrow).

https://codereview.appspot.com/7454047/diff/4001/python2/httplib2test_appengine_aberrant.py
File python2/httplib2test_appengine_aberrant.py (right):

https://codereview.appspot.com/7454047/diff/4001/python2/httplib2test_appengine_aberrant.py#newcode4
python2/httplib2test_appengine_aberrant.py:4: A set of unit tests for

httplib2.py on Google App Engine

Can you make this docstring accurate?

https://codereview.appspot.com/7454047/diff/4001/python2/httplib2test_appengine_aberrant.py#newcode30
python2/httplib2test_appengine_aberrant.py:30:
google.appengine.api.apiproxy_stub_map = None
Can you do this in setUp and fix it in tearDown?

https://codereview.appspot.com/7454047/

[jcgre...@google.com]

https://codereview.appspot.com/7454047/diff/4001/Makefile
File Makefile (right):

https://codereview.appspot.com/7454047/diff/4001/Makefile#newcode8
Makefile:8: cd python2 && python2.7 httplib2test_appengine_aberrant.py

On 2013/03/01 16:33:32, dhermes wrote:
> Still need to run
> python2.7 httplib2test_appengine.py

> Also, kill the tab char (red arrow).

Done.

https://codereview.appspot.com/7454047/diff/4001/python2/httplib2test_appengine_aberrant.py
File python2/httplib2test_appengine_aberrant.py (right):

https://codereview.appspot.com/7454047/diff/4001/python2/httplib2test_appengine_aberrant.py#newcode4
python2/httplib2test_appengine_aberrant.py:4: A set of unit tests for
httplib2.py on Google App Engine

On 2013/03/01 16:33:32, dhermes wrote:
> Can you make this docstring accurate?

Done.

https://codereview.appspot.com/7454047/diff/4001/python2/httplib2test_appengine_aberrant.py#newcode30
python2/httplib2test_appengine_aberrant.py:30:
google.appengine.api.apiproxy_stub_map = None

Not really, as it has to be done before httplib2 is imported.

[dhe...@google.com]

https://codereview.appspot.com/7454047/diff/4001/python2/httplib2test_appengine_aberrant.py
File python2/httplib2test_appengine_aberrant.py (right):

https://codereview.appspot.com/7454047/diff/4001/python2/httplib2test_appengine_aberrant.py#newcode30
python2/httplib2test_appengine_aberrant.py:30:
google.appengine.api.apiproxy_stub_map = None

Ah yes, good call.

Since you're testing the import, can you do that in the test itself?

The reason for the concern, the module 'google.appengine.api' will live
in sys.modules and could have unintended side effects on other tests
that get run. It is very easy to restore this in tearDown, so there is
no harm in doing this.

On 2013/03/01 16:58:18, jcgregorio_google wrote:
> Not really, as it has to be done before httplib2 is imported.

> On 2013/03/01 16:33:32, dhermes wrote:
> > Can you do this in setUp and fix it in tearDown?

https://codereview.appspot.com/7454047/diff/14001/python2/httplib2test_appengine_aberrant.py
File python2/httplib2test_appengine_aberrant.py (right):

https://codereview.appspot.com/7454047/diff/14001/python2/httplib2test_appengine_aberrant.py#newcode2
python2/httplib2test_appengine_aberrant.py:2: httplib2test_appengine
This isn't httplib2test_appengine, it's httplib2test_appengine_aberrant

https://codereview.appspot.com/7454047/

[jcgre...@google.com]

https://codereview.appspot.com/7454047/diff/4001/python2/httplib2test_appengine_aberrant.py
File python2/httplib2test_appengine_aberrant.py (right):

https://codereview.appspot.com/7454047/diff/4001/python2/httplib2test_appengine_aberrant.py#newcode30
python2/httplib2test_appengine_aberrant.py:30:
google.appengine.api.apiproxy_stub_map = None

Done. Moved the test back to httplib2test_appengine.py.

[dhe...@google.com]

LGTM

https://codereview.appspot.com/7454047/

[jcgre...@google.com]

On 2013/03/02 00:30:35, dhermes wrote:
> LGTM

Committed in
https://code.google.com/p/httplib2/source/detail?r=a645d22abfd0cc724f61c8f6c318f7af52047aec


https://codereview.appspot.com/7454047/