ian-kent / MangoX-Queue

Queue implementation using Mango
0 stars 6 forks source link

Compatibility with Mojo::IOLoop->delay #6

Closed oliwer closed 10 years ago

oliwer commented 10 years ago

It is currently impossible to use MangoX::Queue inside a delay(). In this example:

Mojo::IOLoop->delay(
  sub {
    my $delay = shift;
    $queue->enqueue(priority => 1, $job => $delay->begin);
  },
  sub {
    my ($delay, $job) = @_;
    say $job;
  }
)->wait;

in the second sub, $job will be undef (or more exactly, it will contain the error message, if any). That is because in non-blocking enqueue(), you are doing $callback->($job, $error) and delay() will skip the first argument ($job).

There is a convention in Mojo libs when calling a callback : the first argument should always be $self. Since $self is useless in case of a delay(), it is automatically skipped. That's why you should always write $callback->($self, $job, $error).

I realize this is gonna break the API but it is the right thing to do. I may be able to provide a patch if you are lacking time.

ian-kent commented 10 years ago

Thanks for the detailed report :+1: I agree, fixing compatibility with Mojo is more important than maintaining the API, although it should be simple enough to have a backward compatibility flag in for a few versions.

I'll have a look at a fix as soon as I can, but if you are able to provide a patch that would be appreciated.

oliwer commented 10 years ago

Thank you for merging my pull request despite the lack of documentation. Are you planning anything else before releasing to CPAN?

EDIT: oops! Sorry I didn't see there a new release already :+1:

ian-kent commented 10 years ago

No worries :smile: