RapidSMS proposal - simplify configuring which handlers and apps are loaded

[Dan Poirier]

I have a proposal to simplify configuring which handlers and apps are loaded.

Here’s the current situation. RapidSMS loads handlers by looking for a ‘handlers’ directory under each Django app that’s listed in INSTALLED_HANDLERS, or INSTALLED_APPS if INSTALLED_HANDLERS is not defined, but excluding any apps listed in RAPIDSMS_HANDLERS_EXCLUDE_APPS, and any modules listed in EXCLUDED_HANDLERS.  (http://rapidsms.readthedocs.org/en/v0.13.0/topics/contrib/handlers.html#handler-discovery)

This is very complicated. I think it’d be much easier to understand if we just had a single setting to control which handlers are loaded.

Proposal: load the handlers whose full class names (with package) are listed in RAPIDSMS_HANDLERS.  For example:

    RAPIDSMS_HANDLERS = [

"rapidsms.contrib.handlers.KeywordHandler",

"rapidsms.contrib.handlers.PatternHandler",

    ]

This change could be made over several releases, starting by using the RAPIDSMS_HANDLERS setting if it’s there, otherwise using the current behavior and issuing a deprecation warning. In some later release we could clean up the old code and just leave the new setting operative.

We could also do something similar for loading apps.  Currently, RapidSMS loads RapidSMS apps by looking in each Django app for a module named ‘app’ and looking in that for a subclass of AppBase.  We could instead just load the RapidSMS apps whose full class names (with package) are listed in RAPIDSMS_APPS.  Of course, whether we make this change is independent of whether we change how handlers are located.

I’d appreciate feedback on these ideas, whether you have changes to suggest, or think this sounds okay.

Thanks!

Dan Poirier <dpoi...@caktusgroup.com>

[Nic Pottier]

I like this actually, I agree the current setup is pretty shit.  I think this method is also more Django-y as well, similar as to how middle ware is configured.

We could then also stop with all the pre, parse, handle, post nonsense which is rarely used anyways.

So big +1 from me, the sooner the better.

Nic

--
You received this message because you are subscribed to the Google Groups "RapidSMS Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rapidsms-dev...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
 
 

[Dan Poirier]

Does anyone else have any input before we go ahead with this?

Dan P.

[Cory Zue]

Thanks Dan - big +1 from me too.

We could then also stop with all the pre, parse, handle, post nonsense which is rarely used anyways.

Confused about this - how does this change facilitate removing the handling phases?

I'd be concerned about doing that. Agree they're bulky, but also very useful (necessary) for certain types of workflows.

So big +1 from me, the sooner the better.

Nic

On Apr 30, 2013 7:53 PM, "Dan Poirier" <dpoi...@caktusgroup.com> wrote:

I have a proposal to simplify configuring which handlers and apps are loaded.

Here’s the current situation. RapidSMS loads handlers by looking for a ‘handlers’ directory under each Django app that’s listed in INSTALLED_HANDLERS, or INSTALLED_APPS if INSTALLED_HANDLERS is not defined, but excluding any apps listed in RAPIDSMS_HANDLERS_EXCLUDE_APPS, and any modules listed in EXCLUDED_HANDLERS.  (http://rapidsms.readthedocs.org/en/v0.13.0/topics/contrib/handlers.html#handler-discovery)

This is very complicated. I think it’d be much easier to understand if we just had a single setting to control which handlers are loaded.

Proposal: load the handlers whose full class names (with package) are listed in RAPIDSMS_HANDLERS.  For example:

    RAPIDSMS_HANDLERS = [

"rapidsms.contrib.handlers.KeywordHandler",

"rapidsms.contrib.handlers.PatternHandler",

    ]

This change could be made over several releases, starting by using the RAPIDSMS_HANDLERS setting if it’s there, otherwise using the current behavior and issuing a deprecation warning. In some later release we could clean up the old code and just leave the new setting operative.

We could also do something similar for loading apps.  Currently, RapidSMS loads RapidSMS apps by looking in each Django app for a module named ‘app’ and looking in that for a subclass of AppBase.  We could instead just load the RapidSMS apps whose full class names (with package) are listed in RAPIDSMS_APPS.  Of course, whether we make this change is independent of whether we change how handlers are located.

I’d appreciate feedback on these ideas, whether you have changes to suggest, or think this sounds okay.

[Dan Poirier]

On Thu, May 2, 2013 at 9:24 AM, Cory Zue <cz...@dimagi.com> wrote:

We could then also stop with all the pre, parse, handle, post nonsense which is rarely used anyways.

Confused about this - how does this change facilitate removing the handling phases?

I'd be concerned about doing that. Agree they're bulky, but also very useful (necessary) for certain types of workflows

​I'm not sure how removing handling phases would be related to changing how we load handlers either, except that both would be simplifications.​

--
Dan Poirier <dpoi...@caktusgroup.com>