Reloader fixes

[miguelgrinberg]

There are two somewhat related reloader fixes in this PR:

1. When using "python -m werkzeug.serving module:app" to start an app the reloaded app has the path of serving.py in sys.path[0] instead of the current directory. This needs to be fixed to allow the module where the app is to be found.
2. Inspired by the Flask/Click integration I have added support for lazy app loading to run_simple. This is achieved by passing the app to run as a "module:app" string.

Let me know what you think.

Miguel

[miguelgrinberg]

Replaced the empty sys.path[0] hack with code that saves the entire sys.path list and restores it on the subprocess.

[miguelgrinberg]

Rebased to work with the latest reloader changes.

[mitsuhiko]

I generally like that but i would like to switch the cli over to click as well. This is optional behavior and i see now reason why it could not use it.

[untitaker]

Hmm, now I wonder how likely it is that some idiotic code puts non-strings into the syspath and makes the json serialization crash. I don't think we should care for that case, but it is something to consider.

[mitsuhiko]

That's a good point @untitaker for a related reason: json uses unicode. File system paths can be either unicode or bytes. So json dumping is probably going to have absolutely terrible effects.

[untitaker]

There are alternative approaches to this issue mentioned in the outdated diffs. I still think reconstructing the exact command has potential, but using loader for this is definetly too unstable.

[mitsuhiko]

Generally also we need to consider that Flask must have a related problem unless i already worked around it there.

[untitaker]

Maybe we should just pickle the path. The serialized format is only stored for a fraction of a second.

[untitaker]

#461 is a very concrete issue from a end user.

[mitsuhiko]

only pickle the path if we absolutize the items.

[untitaker]

What do you mean by that?

[miguelgrinberg]

@untitaker regarding "I still think reconstructing the exact command has potential", keep in mind on Python 2 the __loader__ object exists only when you start the interpreter with -m, but on Python 3 it always exists, so you need a different way to detect when -m was used, maybe by checking if the main module's name is __main__. It seemed to me the code would be full of exceptions and corner cases, all working on barely documented or undocumented features.

[untitaker]

the loader object exists only when you start the interpreter with -m

I only need it when the interpreter was started with -m. Checking whether the interpreter was started with -m could be done like in the first iteration of your PR: Checking whether sys.path[0] == ''.

if the main module's name is main

Isn't that always the case?

[miguelgrinberg]

Checking whether the interpreter was started with -m could be done like in the first iteration of your PR: Checking whether sys.path[0] == ''

Sure, but we are back to undocumented behavior. Are we sure this isn't ever going to change?

if the main module's name is main

Isn't that always the case?

Yes, if you look at the value of __name__, but this is what I came up when testing on Python 3.4. This is my test script:

from __main__ import __loader__
print(__loader__.name)

Here is what happens when you run it directly vs. with -m:

(venv) $ python test.py
__main__
(venv) $ python -m test
test

But I still think this is too obscure to use.

[untitaker]

Sure, but we are back to undocumented behavior. Are we sure this isn't ever going to change?

AFAICT the fullname attribute isn't documented either, the ABC doesn't define it. Not sure we could ever come up with something documented if we went down that route. Maybe we should stick to copying path over, but as i said, i am not sure what @mitsuhiko meant with absolutizing the items. os.path.abspath is the only thing that comes to mind, but i can't see why it is specifically required when used with pickling.

[mitsuhiko]

The loader API is so fundamentally broken and changes all the time, i would stay away from it.

By absolitize i mean [os.path.abspath(x) for x in sys.path].

[untitaker]

Bump, @miguelgrinberg could you serialize the path using pickle and apply abspath before?

[miguelgrinberg]

Sorry, I dropped the ball on this. I'll change to pickle, that's not a problem. @mitsuhiko did not explain why it is necessary to make the sys.path elements absolute. Are there any use cases where the subprocess will not start on the same cwd as the parent?

[miguelgrinberg]

@untitaker the pickling appears to work fine, but I'm seeing a strange bug in Python 3.3 and 3.4, so there is still some work before this can be used.

As we discussed, when starting the app with -m werkzeug.serving the sub-process has the directory where serving.py is in sys.path[0]. At some point before we restore the path from the pickled env var someone tries to import something from http.client from the standard library, but in Python 3 the http.py file from werkzeug is picked up instead, due to being in the same directory as serving.py, so it shows up first in sys.path. The process exits with 'http' is not a package.

Need to think about how to handle this, it seems we need to restore sys.path first thing after the sub-process starts. If you have any ideas let me know.

[miguelgrinberg]

Logged this issue as #593, since it is unrelated to this one.

[untitaker]

I played around with this a bit and i can't see a way how we could solve this problem by copying sys.path at all. Even if we manage to replace the whole search path before any imports are processed (which i tried by setting PYTHONPATH in new_environ), issues like #461 would still stick around. It seems to me that all these problems really could be avoided by trying to recreate the original command (i.e. using __loader__).