-
-
Notifications
You must be signed in to change notification settings - Fork 495
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
Fixed constantize issue #2243
Fixed constantize issue #2243
Conversation
@@ -32,7 +32,7 @@ def record(queue, worker, payload, &block) | |||
|
|||
finish_transaction(transaction, 200) | |||
rescue Exception => exception | |||
klass = payload['class'].constantize | |||
klass = payload['class'].to_s.constantize |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the class attribute is already a class, then we don't want to re-constantize it.
klass = payload['class'].to_s.constantize | |
klass = payload['class'] | |
if klass.is_a(String) | |
klass = klass.constantize | |
end |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #2243 +/- ##
==========================================
+ Coverage 97.41% 97.43% +0.02%
==========================================
Files 102 102
Lines 3823 3825 +2
==========================================
+ Hits 3724 3727 +3
+ Misses 99 98 -1
|
Thanks for the PR. Is it possible to write a test case for this fix? |
Tests and fixes getsentry#2243
There are already tests covering this but they are not catching this due to |
Tests and fixes getsentry#2243
* Allow custom Redis instance for tests You probably don't want the tests to use the same Redis you use for any app development. Maybe you want to use Docker with custom port: docker run --rm -it -p 16379:6379 redis:6 * Run sentry-resque specs without Rails Tests and fixes #2243 * Run with latest `psych` I think ruby/psych#655 was resolved long ago * Note `constantize` sentry-resque fix in CHANGELOG
Thanks for your Pull Request 🎉
Please keep these instructions in mind so we can review it more efficiently:
Other Notes
Description
Describe your changes:
Fixed
constantize
issuegot #<NoMethodError: undefined method
constantize' for MyWorker:Class`