Importing top-level webapp2 breaks encapsulation #302

Activity

1. dhermes commented on Sep 8, 2015

   

   dhermes

   on Sep 8, 2015

   Contributor

   More actions

   @tseaver How do you mean it breaks encapsulation? The oauth2client.appengine module isn't imported in the package, so a user needs to explicitly say "import me some appengine, I'm on the appengine".
2. tseaver commented on Sep 8, 2015

   

   tseaver

   on Sep 8, 2015

   Author

   More actions

   The gcloud tests covering the "appengine detection" machinery mock out the necessary google.appengine bits to allow oauth2client.appengine to import without breaking. f3f8b4f breaks that, switching to import webapp as an alias for an entirely new top-level webapp2 package (newly-imported here, at least).

   I don't know what the motivation for f3f8b4f was, but it caused the gcloud tests to break today due to the ues of newly-released oauth2client 1.5.0.
3. dhermes commented on Sep 8, 2015

   

   dhermes

   on Sep 8, 2015

   Contributor

   More actions

   Motivation: webapp2 replaced webapp in either 2011 or 2010, so the code was very old.

   I realize it hurt our test mocks in a library elsewhere, but that isn't an issue with oauth2client.

   I'm closing out, let me know if you think there is something left in oauth2client that needs to be resolved.
4. dhermes

   closed this as completedon Sep 8, 2015
5. tseaver commented on Sep 8, 2015

   

   tseaver

   on Sep 8, 2015

   Author

   More actions

   FTR, the commit was a fix for #217. That change added an import dependency on webapp2, without also adding as an install_requires dependency in setup.py: AFAICT, it should be there (d2c15e6 patched it into tox.ini for the docs). Perhaps it should be in an 'extras_require`, e.g.:

   setup(
   #...
       extras_require={'appengine': ['webapp2', 'appengine']},
   )
6. dhermes commented on Sep 8, 2015

   

   dhermes

   on Sep 8, 2015

   Contributor

   More actions

   App Engine is a nasty beast. Users deploy code there and all of these dependencies are just there. (webapp2 is luckily open source and available on PyPI)
7. tseaver

   added a commit that references this issue on Sep 9, 2015

   Import webapp2 instead of google.appengine.ext.webapp

   ...

   f3f8b4f