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

Fix ES compatibility for sidekiq-unique-jobs #6

Merged
merged 2 commits into from
Aug 23, 2018

Conversation

matus-vacula
Copy link
Contributor

@iMacTia as discussed, here is the fix to support sidekiq-unique-jobs

@iMacTia
Copy link
Owner

iMacTia commented Apr 11, 2018

@matus-vacula thanks for this new PR.
Are you sure there are no other custom fields introduced by sidekiq-unique-jobs that may have similar issues? Worth running a little research and fix all of them in this PR if possible.

In the meantime, I'm adding @silviusimeria on this one as well for review

@silviusimeria
Copy link
Collaborator

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

@silviusimeria
Copy link
Collaborator

@iMacTia after checking out sidekiq-unique-jobs it looks like the unique_args is the only key introduced that has a more complex data structure (Array) and needs to be stringified for ES.

However the unique_args will have the same bug as the args regarding #3 , but that can be fixed separately.

Copy link
Owner

@iMacTia iMacTia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @silviusimeria, happy to merge this in in that case!

@iMacTia iMacTia merged commit e4e2457 into iMacTia:master Aug 23, 2018
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

Successfully merging this pull request may close these issues.

3 participants