Convert 'retry' to string for ES compatibility

[matus-vacula]

'retry' field can contain not just integer but also false. This breaks ES compatibility.

The error I'm currently getting:

failed to parse [retry]
Current token (VALUE_FALSE) not numeric, can not use numeric value accessors

[vitorreis]

The fix is appreciated

[matus-vacula]

@iMacTia please check

[iMacTia]

Hi @matus-vacula, retry should always be a boolean, while retry_count is a number.
Can you show me an example of retry being a number and from which log that comes from?
I can't just accept this PR and merge it, as it will break others ES setup

[matus-vacula]

Hi @iMacTia, as mentioned in https://github.com/mperham/sidekiq/blob/master/lib/sidekiq/job_retry.rb#L26 it can contain an integer too.

[iMacTia]

I see, that makes sense then.
It seems to be the case when you specify a max number of retries per job.
I'd then suggest we do a type check on retry and, if Integer, we add it to the payload at a separate voice. Something like:

if payload['retry'].is_a?(Integer)
  payload['max_retries'] = payload['retry']
  payload['retry'] = true
end

This way backwards compatibility should be preserved.
Would this work for you?

[matus-vacula]

Works for me.

[matus-vacula]

@iMacTia updated

[iMacTia]

Thanks @matus-vacula, looks good to me, I'd just like another maintainer to have a look as I'm not currently using this in production, but they are.
I've added him as a reviewer, if he's happy with the change as well, I'll get it merged and release a new version

[matus-vacula]

I stumbled upon another case where delivery to ES was broken. unique_args can have the same problem as args

[iMacTia]

@matus-vacula I think unique_args is not a standard Sidekiq field.
Are you using sidekiq-unique-jobs?

If that's the case, then probably all fields injected by that gem are currently not supported.
I'd suggest to keep this PR simple and only fix the retry which is a standard Sidekiq property.

Support for sidekiq-unique-jobs should be addressed separately upon further investigation on a separate PR

[matus-vacula]

sure, no problem

[iMacTia]

Looks good to me, if good for @silviusimeria as well we can merge it 👍

[silviusimeria]

@iMacTia sorry, I have just noticed this, I will have a look

[silviusimeria]

Looks good to me