Skip to content

Conversation

@lucasmazza
Copy link
Contributor

This PR has some of the initial code to add support for Active Job subclasses regardless of the queue backend that is being used - Raven currently supports Sidekiq on it's own but would be nice to have a broader support for other backends like SuckerPunch (the one I'm using), Resque, DelayedJob, etc.

I couldn't find any tests for the Sidekiq support to use as a base, but I'm take this to a spin in our app to see how it goes. Also, I fear that loading both the ActiveJob support + Sidekiq might generate duplicate reports since both integrations would be executed together, but this is something that I need to test it out to be sure about it.

@nateberkopec
Copy link
Contributor

👍 We should probably only carry ActiveJob integration in the base gem, and spin out Sidekiq integration to a separate gem.

@lucasmazza
Copy link
Contributor Author

@nateberkopec one alternative that I can think of is make it disabled by default - the deprecation cycle/upgrade path could be simpler.

@lucasmazza
Copy link
Contributor Author

@nateberkopec sorry for the huge delay on this one. I've tested this branch against a fresh Rails app using Sidekiq and added a safeguard to the ActiveJob plugin to let the Sidekiq handle the exceptions instead of collecting the same error twice.

nateberkopec added a commit that referenced this pull request Jul 14, 2015
@nateberkopec nateberkopec merged commit ec22b8f into getsentry:master Jul 14, 2015
@dcramer
Copy link
Member

dcramer commented Sep 22, 2015

I've reverted this for now until we sort out #368.

It might be that as a quick workaround we remove the clause to prevent the duplicate and sort out those problems later.

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