AppEngine environments: Flask ExtensionImporter class (flask.ext) is getting in the way when flask extensions import external modules #662

Closed
irvingpop opened this Issue Jan 6, 2013 · 6 comments

Projects

None yet

5 participants

@irvingpop

Several people have noted issues when installing/updating Flask extensions that use the new style flask.ext path, specifically in Google AppEngine environments.

Relevant discussion and troubleshooting here: thadeusb/flask-cache#38

What is happening (my understanding):

  1. flask extension (ex: Flask-Cache) imports a module (ex: "from jinja2 import nodes")
  2. in the extension's context, ExtensionImporter class has added its own path (flask.ext) to sys.meta_path
  3. When ExtensionImporter load_module() is run, it fails to load the external module (with an ImportError?) and thus bombs

Troubleshooting notes:

  • this may be caused by AppEngine's dev_appserver_import_hook.py, line 1577, in LoadModuleRestricted
  • The application will run normally if you comment out line 86 in flask/exthook.py "raise ImportError('No module named %s' % fullname)" or by reverting to the the old flaskext path

Any assistance is highly appreciated.

@irvingpop irvingpop referenced this issue in thadeusb/flask-cache Jan 6, 2013
Closed

Support GAE #38

@thadeusb

To cause the problem, the extension must be a package, and attempt to import another file from within that package.

Given

flask_mything/__init__.py
flask_mything/util.py

If an import statement in the init.py file is defined as

from flask.ext.mything.util import some_useful_function

imports within util will fail.

This "can" be fixed from the extension by using relative imports instead of absolute. Such as:

from .util import some_useful_function 

will cause the extension to load properly.

However, the proper thing for Flask to do is to not raise an exception, it should return None as is required in the python documentation -> http://docs.python.org/2/library/sys.html#sys.meta_path (The method returns None if the module cannot be found, else returns a loader.)

@techniq

Has anyone tried using the experimental devappserver2 (http://code.google.com/p/appengine-devappserver2-experiment/) and see if it still has the same issues?

@mitsuhiko
The Pallets Projects member

I think that is a GAE issue. If someone can reproduce that without GAE please reopen that ticket. The fix for GAE would be to not use the rename imports for the actual module names.

@mitsuhiko mitsuhiko closed this Jan 27, 2013
@techniq

There is a GAE issue open for this here - http://goo.gl/rrCGh

@thadeusb

I fail to see how this is a GAE issue when Flask's meta path importer is doing the wrong thing. Flask would fail in other systems that added their own importers to sys.meta_path.

sys.meta_path
A list of finder objects that have their find_module() methods called to see if one of the objects can find the module to be imported. The find_module() method is called at least with the absolute name of the module being imported. If the module to be imported is contained in package then the parent package’s path attribute is passed in as a second argument. The method returns None if the module cannot be found, else returns a loader.

sys.meta_path is searched before any implicit default finders or sys.path.

See PEP 302 for the original specification.

@apiguy

Flask's find_module() method from the ExtensionImporter behaves as it should. Here it is:

def find_module(self, fullname, path=None):
    if fullname.startswith(self.prefix):
        return self

Seems like it's returning a loader (or None, if the prefix doesn't match) as the documentation says it should. By the way, the documentation references the PEP because there is more information in there. Here is a more complete version of what should happen:

http://www.python.org/dev/peps/pep-0302/#specification-part-1-the-importer-protocol

As far as I can see, the only place where Flask doesn't meet PEP 302 is in load_module() setting the attributes like __file__ and such explicitly which is probably OK since it uses __import__() to perform the actual import.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment