Skip to content
This repository was archived by the owner on Apr 29, 2019. It is now read-only.

Conversation

@tacaswell
Copy link
Contributor

First pass at factoring out the code that fixes the up failed service
request-id.

The Chicago specific code has been moved to fix_request_id_chi, which
is assigned to fix_request_id, which is the function that is actually
called in show_request(). Any other cities need to write an
equivalent to fix_request_id_chi and assign it to fix_request_id
(presumably, there is a way to make this more pluggable/configurable).

First pass at factoring out the code that fixes the up failed service
request-id.

The Chicago specific code has been moved to fix_request_id_chi, which
is assigned to fix_request_id, which is the function that is actually
called in show_request().  Any other cities need to write an
equivalent to fix_request_id_chi and assign it to fix_request_id
(presumably, there is a way to make this more pluggable/configurable).
@Mr0grog
Copy link
Member

Mr0grog commented Jan 17, 2013

This looks like a pretty good start. Thanks for taking a crack at it! I like how you are reassigning the function to a more generic name.

Looking at this, I'm thinking having a separate file full of methods like this one that could be called by the main application would be a good model. It could either have a standard name (maybe localized_behaviors.py?) or you could specify the name of the file to use via a config var.

So you might have this folder structure:

/
  - app.py
  - config.py
  - localized_behaviors.py

And app.py might have:

try:
  import localized_behaviors as hooks
except:
  hooks = None

or

hooks = None
if 'localized_behaviors' in app.config:
  hooks = importlib.import_module(app.config['localized_behaviors'])

Any thoughts?

@Mr0grog
Copy link
Member

Mr0grog commented Jan 17, 2013

Also, in either of these cases, I think we'd either ship Chicago's as the example or have a separate branch that includes the Chicago version of the methods in localized_behaviors (or whatever we call it).

@tacaswell
Copy link
Contributor Author

I left it the way I did because I have no experience writing pluggable stuff like this. I think from your comment I should be able to get something working.

@Mr0grog
Copy link
Member

Mr0grog commented Jan 17, 2013

Great! sounded like that might be the case. Always happy to provide pointers. You can also check out get_notifiers() in updater/update.py for a full-on example of the complicated way to do it (it needs to not have some of the standard machinery run).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants