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 Verk.Job decode/1 #201

Merged
merged 1 commit into from
Jun 22, 2020
Merged

Fix Verk.Job decode/1 #201

merged 1 commit into from
Jun 22, 2020

Conversation

alissonsales
Copy link
Collaborator

@alissonsales alissonsales commented Jun 22, 2020

It should only care about Verk.Job struct keys discarding any unknown keys.

This is a safer implementation for users that schedule jobs to Verk
using ruby Sidekiq::Client.

A recent version of newrelic_rpm gem is injecting code into Sidekiq
jobs for some sort of tracing/inspection making jobs scheduled with
Sidekiq::Client invalid to Verk as they contain an unknown job key.

@coveralls
Copy link

coveralls commented Jun 22, 2020

Coverage Status

Coverage remained the same at 88.312% when pulling d3b243f on fix-job-decode into 79b0e79 on master.

It should only care about job keys discarding any unknown keys.

This is a safer implementation for users that schedule jobs to Verk
using ruby Sidekiq::Client.

A recent version of `newrelic_rpm` gem is injecting code into Sidekiq
jobs for some sort of tracing/inspection making jobs scheduled with
Sidekiq::Client invalid to Verk as they contain an unknown job key.
Copy link
Owner

@edgurgel edgurgel left a comment

Choose a reason for hiding this comment

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

LGTM

@edgurgel edgurgel merged commit 75e14c0 into master Jun 22, 2020
@alissonsales alissonsales deleted the fix-job-decode branch June 22, 2020 20:53
@alissonsales
Copy link
Collaborator Author

@edgurgel thanks :)

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