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

Job::recreate() encapsulates args array inside another array #102

Closed
CyrilMazur opened this issue Apr 15, 2013 · 8 comments
Closed

Job::recreate() encapsulates args array inside another array #102

CyrilMazur opened this issue Apr 15, 2013 · 8 comments

Comments

@CyrilMazur
Copy link
Contributor

Job::recreate is calling Job::create, which enqueues the job like this

Resque::push($queue, array(
'class' => $class,
'args' => array($args),
'id' => $id,
));

It encapsulates array($args) (don't really know why?)

Anyways, if you call recreate, that will pass array($args) to create() and then encapsulate into another array. So if you all recreate() once, you end up with

array(array($args))

if you call it twice, you end up with

array(array(array($args)))

and so on... thus when performing a job that was enqueued using recreate(), the $args array is wrongly encapsulated into another array.

I fixed this by changing the line 234 of Job.php like this:

return self::create($this->queue, $this->payload['class'], $this->payload['args'][0], $monitor);

(jus added [0] to $this->payload['args'])

@ruudk
Copy link
Contributor

ruudk commented Apr 16, 2013

A PR would be better :)

@danhunsaker
Copy link
Contributor

The trouble with a pull request at this point is the question of why $args is being wrapped in an(other) array. Should it, instead, be cast to an array? Knowing why it is wrapped will determine which of the two solutions should be submitted in a PR to begin with.

@ruudk
Copy link
Contributor

ruudk commented Apr 16, 2013

True, I found out this:

The encapsulated array() call in Job::create was added 2 years ago in order to be better compatible with the Ruby version: f7eac3b

When you look at Job->getArguments() you'll see that it then returns the first index of the array: https://github.com/chrisboulton/php-resque/blob/master/lib/Resque/Job.php#L128

So this should be correct:

return self::create($this->queue, $this->payload['class'], $this->payload['args'][0], $monitor);

@ruudk
Copy link
Contributor

ruudk commented Apr 16, 2013

Maybe $this->getArguments() would be better as it returns an empty array when there are no arguments given. Where $this->payload['args'][0] could throw an undefined index error.

@danhunsaker
Copy link
Contributor

You make it sound like git keeps a record of changes, and if commit messages are actually thoughtfully written, others can go back and see why things are done a certain way! A technology like that would be.... Well, useful.

Said another way: Oh. Right. The answer is probably in the commit history somewhere, and something like git blame would probably help narrow down where. Duh. Should have thought of that.

@CyrilMazur
Copy link
Contributor Author

yeah $this->getArguments() would be the proper way to do it, like you said, in case $this->payload['args'][0] is undefined. Pull request sent, thanks guys.

@Rockstar04
Copy link
Contributor

It looks like this issue could be closed? Fixed in #104

@CyrilMazur
Copy link
Contributor Author

True @Rockstar04

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

4 participants