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

PerformableMethod#method namespace clash with Delayed::Backend::Base::ClassMethods#hook #113

Closed
guns opened this issue Sep 7, 2010 · 4 comments

Comments

@guns
Copy link

guns commented Sep 7, 2010

PerformableMethod has a reader method #method which overrides Object#method.

Normally, this is not a problem, but if you mixin a failure hook into the PerformableMethod class, when the hook procedure tries to call Object#method on the payload_object (to determine it's arity), it raises a NameError when the payload object is a PerformableMethod.

I know that augmenting PerformableMethod isn't really part of the API, but I imagine that you're planning on adding an interface for adding failure hooks for method.delay, since creating a custom job class for every method you desire to have a failure hook is a little cumbersome.

I had a branch cooking that fixed the issue by renaming PerformableMethod#method to #delayed_method (overriding the core #method was a poor choice IMHO), and was starting on an API for adding custom failure methods via inline Procs or Modules: method.delay(:failure => lambda {}).method(args)

However, you have been busy recently! delayed_job is undergoing a major overhaul and my branch has become a mess to integrate. So instead of trying to read your mind, I'd like to ask if you could resolve this issue.

@bkeepers
Copy link

bkeepers commented Sep 9, 2010

Ah, thanks for finding this. I'll pull your fix and add specs for it.

@guns
Copy link
Author

guns commented Sep 9, 2010

Well, my old branch is a dead-end. This branch has the simple rename:

http://github.com/guns/delayed_job/tree/issue-113

I was hoping to implement the inline #failure Proc, but I didn't see an obvious way to get around the fact that Procs aren't serializable. Specifying a module: object.delay(:failure => MyFailureModule).method could work by simply extending the payload_object with the module after deserializing it.

Also, note that renaming PerformableMethod#method breaks the backend specs, or at least they did while I was trying to fix them in this branch:

http://github.com/guns/delayed_job/tree/on-permanent-failure-hooks-for-performable-methods

@bkeepers
Copy link

bkeepers commented Sep 9, 2010

Fix hooks on PerformableMethod and PerformableMailer [Closed by e132b5d]

@bkeepers
Copy link

bkeepers commented Sep 9, 2010

@guns: Yeah, the proc thing is a tough one. For now I made it so that the hooks are delegated to the object that the method is called on.

This issue was closed.
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