Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Callbacks. #6

Closed
pote opened this issue Jul 13, 2015 · 3 comments
Closed

Callbacks. #6

pote opened this issue Jul 13, 2015 · 3 comments

Comments

@pote
Copy link
Owner

pote commented Jul 13, 2015

We're discussing the possible callbacks implementation with @foca and thought it'd be nice to get opinions from the rest.

Current implementation

There are currently 3 callbacks in Disc: the first one is an error handling callback, which is global to Disc and is executed whenever an unhandled exception is raised while running a job, the other two are lifecycle callbacks: disc_start and disc_done, those are specific to a job class and executed before and after the perform method respectively.

My proposal

I am really wary of callbacks and I particularly dislike things that promote an overly composable callback style as they are easy to abuse and it becomes common to not have any idea of what is going on in a particular piece of code when using them, the intent of the current implementation is to be able to easily share callbacks between groups of jobs via mixins, for example.

So what I've been thinking is: why not just limit lifecycle callbacks to be global like the error handling one? That way all you need to do in order to know what's going on in a job is to look at the job itself and the disc_init file, moreover: the use cases we've been thinking about for these callbacks are about logging and metrics, stuff that you would want to define globally.

So I think having something like Disc.on_start and Disc.on_done (naming not necessarily that) would be nicer, it is of course a bit more limited, but it would allow for the use cases we're considering and enforce that things either happen there or on perform methods, which should make things easier to reason about.

Anyway, we wanted to open up the discussion and hear your thoughts. Any opinions are appreciated. :)

@pote
Copy link
Owner Author

pote commented Jul 13, 2015

@foca anything I'm missing?

@foca
Copy link
Collaborator

foca commented Jul 14, 2015

No, I think that sums it up nicely.

@pote
Copy link
Owner Author

pote commented Jul 29, 2015

I'm closing the issue for the time being, as I'm not sure we even really need callbacks right now, we'll likely reopen when the need arises.

@pote pote closed this as completed Jul 29, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants