-
Notifications
You must be signed in to change notification settings - Fork 131
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
Add support for Sneakers #676
Conversation
I think there's no harm in making it experimental for now. But I would also write a comment on the Sneakers PR you referenced to let them know you're hoping that it'll be merged soon. That seemed to work for this jondot/sneakers#367 |
Codecov Report
@@ Coverage Diff @@
## master #676 +/- ##
===========================================
+ Coverage 64.11% 95.16% +31.04%
===========================================
Files 99 103 +4
Lines 2943 3205 +262
===========================================
+ Hits 1887 3050 +1163
+ Misses 1056 155 -901
Continue to review full report at Codecov.
|
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.
There's a minor version mistake in the docs
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.
another minor comment
Co-Authored-By: Emily S <emily.s@elastic.co>
Continued from #599
Updating the implementation and spec a little bit. I don't feel we need to include version checking in the spec unless explicitly testing with different versions.
@redwrx has an open PR here (jondot/sneakers#414) to pass the worker itself to middleware. Until that is merged I don't think there's a way to get the worker name for the transaction names. Too bad, and we fall back on the queue name.
@estolfo, how do you feel about marking Sneakers support as experimental until a way to get the worker name is provided? That way we won't need to do the whole version dance around breaking changes when we change the names of all Sneakers transactions?