ChristopherRabotin / bungiesearch

UNMAINTAINED CODE -- Elasticsearch-dsl-py django wrapper with mapping generator
BSD 3-Clause "New" or "Revised" License
67 stars 20 forks source link

Add ability to create custom signal processing classes #79

Closed afrancis13 closed 9 years ago

afrancis13 commented 9 years ago

The ability to create and utilize custom signal processing classes is something I needed given the complexity of the signal processing (and it's relationship with task management) that we currently support. I have packaged the signal processing functions into a default class called BungieSignalProcessor and configured the managers document to reflect these changes. The user can now create their own signal processor and use it with Bungiesearch by placing the key 'SIGNAL_CLASS' (and the string value of the module for signal processing) in settings['SIGNALS']. The default behavior without this key-value pair is the use of BungieSignalProcessor (and, as before, if signals is not defined in settings, no automatic updates). I've tried my best to reflect these changes as concisely as possible in the README.

ChristopherRabotin commented 9 years ago

Yet another great PR! Thanks @afrancis13 ! I'm wondering, while we're at it [changing the signal processing], could it be of interest to allow for multiple signals to be connected. I'm trying to think of a proper use case, although Django does support, by itself, attaching multiple hooks to a given signal.

Any thoughts?

ChristopherRabotin commented 9 years ago

Another comment on the PR actually, would it be possible to add some tests for the signal processing to make sure that when it's specified, it will be executed?

afrancis13 commented 9 years ago

@ChristopherRabotin thanks for the feedback, fixes have been made. I wrote an uncreative test, and did find a bug, so I cleaned that up. As for the comment about multiple signals, I probably won't pursue that immediately, but perhaps put up an issue?

ChristopherRabotin commented 9 years ago

Great! I'll review that in the beginning of next week (busy until then). As for the multiple signals, yes, definitely open an issue as a question and we can assess later.

Best regards, Christopher Rabotin. On Jun 18, 2015 9:53 PM, "Alex Francis" notifications@github.com wrote:

@ChristopherRabotin https://github.com/ChristopherRabotin thanks for the feedback, fixes have been made. I wrote an uncreative test, and did find a bug, so I cleaned that up. As for the comment about multiple signals, I probably won't pursue that immediately, but perhaps put up an issue?

— Reply to this email directly or view it on GitHub https://github.com/Sparrho/bungiesearch/pull/79#issuecomment-113286534.

afrancis13 commented 9 years ago

Good catches once again - apologies for missing the print statements!

ChristopherRabotin commented 9 years ago

No worries, that's why we have pull requests! ;)

ChristopherRabotin commented 9 years ago

Yet again, many thanks for this PR @afrancis13 !