Kitware / tangelo

A simple, quick, powerful web framework
http:/tangelohub.org/tangelo/
Apache License 2.0
185 stars 35 forks source link

Added watch plugin #528

Closed manthey closed 9 years ago

manthey commented 9 years ago

Originally, only modules directly loaded by service calls would be reloaded if they (or a companion config file) changed. This functionality missed reloading other modules that changes.

It was deemed that it really should be in a plugin rather than in the core service. Now, if the watch plugin is not included, Tangelo doesn't monitor if modules and config files are altered. If the watch plugin is enabled (either via --watch on the command line or by including it in the plugins), the original monitoring function takes place. Additionally, any service or python module that has changed or has a dependency that has changed will be reloaded.

There are some potential reload issues that are intrinsic to python and some potential reload issues that are a result of trying to automate the entire process:

What we could work around: Suppose service A imports module B which imports module C which imports module D. Module D is then altered. Service E is now loaded which imports module C, we'll see that module D has changed and reload module D BUT we won't reload modules B or C until service A is called again. This means that in module B or C does something different because of a change to module D, we may not get the whole change in Service E.

The work-around would be that for every import, we would have to check each use of the imported module and check if any of its children need to be updated. If so, we first have to reload the children and then reload the ancestors in a robust order. This still may not do what we want, as we don't know which submodules that rely on module D should be deferred until a service needs them rather than reloaded with service E. For instance, module B might be something that must not be restarted until something like module A imports it.

The usual python reload has some issues as well. See the python documentation for these issues.

All of that being said, this should usually result in the expected action at the expected time.

This still needs the help files updated.

This will resolve issue #516.

waxlamp commented 9 years ago

My inuition is that, because of complexity of doing it right vs. what we gain, we should not worry about the workaround you described. If the watch plugin covers the most common use cases, I think that's good enough, since this is a convenience for development and in fact doesn't guarantee anything about production behavior. What I mean is, if the watch plugin doesn't automatically cover some particularly byzantine structure of interdependent plugins, the developer will in any case want to verify that things work properly by actually stopping and starting Tangelo. It's sort of a tradeoff for that developer between having watch functionality and sparing whatever performance losses occur from stopping and restarting Tangelo. That developer is a special case - for most developers, the tradeoff won't exist.

waxlamp commented 9 years ago

This is really nice. Thanks for the effort @manthey! And let me know if this LGTY.

As with the others, the original PR is saved at the watch-scripts-SAVE branch.

manthey commented 9 years ago

@ronichoudhury I had a few issues with module dependencies that I've fixed. I expanded the tests to check this. I updated the documentation. I think it is now fully ready.

waxlamp commented 9 years ago

Thanks @manthey!