Cross platform config auto reload #326

Open
fxstein opened this Issue Aug 17, 2015 · 4 comments

Comments

Projects
None yet
2 participants
@fxstein
Contributor

fxstein commented Aug 17, 2015

First of all thanks for adding support (even if experimental) for the auto config reload!

Unfortunately pyinotify is not supported on the likes of OS X, limiting the use of the feature for those of us developing on OS X. Not only can one not install pyinotify on OS X, once you register

extensions = ['reload_config']

without conditional OS logic, an app will error out on OS X, because of the dependency missing.

Have you maybe looked at watchdog as a cross platform alternative?

Thanks a lot for all the work you are putting into this!

@derks

This comment has been minimized.

Show comment
Hide comment
@derks

derks Aug 21, 2015

Member

@fxstein thanks for the note. I've not looked into Watchdog before... but will definitely check it out as a possible alternative.

Member

derks commented Aug 21, 2015

@fxstein thanks for the note. I've not looked into Watchdog before... but will definitely check it out as a possible alternative.

@derks derks added this to the 2.8.0 Stable milestone Dec 4, 2015

@derks derks self-assigned this Dec 4, 2015

@derks

This comment has been minimized.

Show comment
Hide comment
@derks

derks Feb 9, 2016

Member

I've begun playing around with watchdog a bit and like it. Now that app.reload() is available in cement/master (2.8.x) I will be refactoring the ext_reload_config extension into a more general ext_watchdog that will be more useful than simply watching config files.

My thoughts are:

from cement.core.foundation import CementApp

def reload_app_if_config_changes(app, modified_path):
    if modified_path in app._meta.config_files:
        app.reload()

class MyApp(CementApp)
    class Meta:
        extensions = ['watchdog']

with MyApp() as app:
    for path in app._meta.config_files:
        app.watchdog.add(path)

    app.hook.register('watchdog_path_modified', reload_app_if_config_changes)
    app.run()

This is just a mockup... but essentially... A) Ability to add files/dirs to watch, and B) tie in with hooks that can operate if anything chnages (that we're watching).. and if those are config files, call app.reload().

Not sure how this will work... I tried with ext_reload_config and got thread/signal issues trying to call app.reload() from a hook... but not sure if that's something in how I implemented pynotify.

Will try and work on this this week, but might not make it into 2.8.x.

Member

derks commented Feb 9, 2016

I've begun playing around with watchdog a bit and like it. Now that app.reload() is available in cement/master (2.8.x) I will be refactoring the ext_reload_config extension into a more general ext_watchdog that will be more useful than simply watching config files.

My thoughts are:

from cement.core.foundation import CementApp

def reload_app_if_config_changes(app, modified_path):
    if modified_path in app._meta.config_files:
        app.reload()

class MyApp(CementApp)
    class Meta:
        extensions = ['watchdog']

with MyApp() as app:
    for path in app._meta.config_files:
        app.watchdog.add(path)

    app.hook.register('watchdog_path_modified', reload_app_if_config_changes)
    app.run()

This is just a mockup... but essentially... A) Ability to add files/dirs to watch, and B) tie in with hooks that can operate if anything chnages (that we're watching).. and if those are config files, call app.reload().

Not sure how this will work... I tried with ext_reload_config and got thread/signal issues trying to call app.reload() from a hook... but not sure if that's something in how I implemented pynotify.

Will try and work on this this week, but might not make it into 2.8.x.

@fxstein

This comment has been minimized.

Show comment
Hide comment
@fxstein

fxstein Feb 9, 2016

Contributor

Awesome! I will look at it this week as well to see how this would work. Thank you!

Contributor

fxstein commented Feb 9, 2016

Awesome! I will look at it this week as well to see how this would work. Thank you!

@derks derks added the 1 - Ready label Feb 12, 2016

@derks derks removed the stable/2.10.x label Feb 25, 2016

@derks derks removed this from the 2.8.0 Stable milestone Feb 25, 2016

@derks derks added 0 - Backlog and removed 1 - Ready labels Jul 12, 2016

derks added a commit that referenced this issue Jul 13, 2016

@derks

This comment has been minimized.

Show comment
Hide comment
@derks

derks Jul 13, 2016

Member

@fxstein I've added the watchdog extension (See Issue #394) which sets us up to properly re-implement the reload_config extension. With regards to reloading the application when configuration files are modified is a little more complicated, but I've got a working prototype I'll be trying to post here later today.

There are some caveats. The first being that with the default list of config files, we'd end up watching the users entire home directory because of the default path of ~/.myapp.conf (you can't watch files, only directories). That is obviously a no go and in my test it actually hung the process and never actually executed the controller. The solution to that is to not include that config file path.

Will be working on a proper example... which may or may not replace the reload_config. There's a possibility that we'll be better off just providing an example doc of how you might do it... but not enforce an opinionated way of doing it via an extension.

Member

derks commented Jul 13, 2016

@fxstein I've added the watchdog extension (See Issue #394) which sets us up to properly re-implement the reload_config extension. With regards to reloading the application when configuration files are modified is a little more complicated, but I've got a working prototype I'll be trying to post here later today.

There are some caveats. The first being that with the default list of config files, we'd end up watching the users entire home directory because of the default path of ~/.myapp.conf (you can't watch files, only directories). That is obviously a no go and in my test it actually hung the process and never actually executed the controller. The solution to that is to not include that config file path.

Will be working on a proper example... which may or may not replace the reload_config. There's a possibility that we'll be better off just providing an example doc of how you might do it... but not enforce an opinionated way of doing it via an extension.

@derks derks removed the 1 - Ready label Jan 17, 2018

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