-
Notifications
You must be signed in to change notification settings - Fork 309
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 tedious integration #599
Conversation
ugh, this PR is so tedious.... sorry, couldn't help myself. |
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.
Request cancellation is not tested.
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.
Bulk loading / streaming is not tested.
Yes, this is still a work in progress. |
Cancelling requests behaviour changed in v6.2.0: https://github.com/tediousjs/tedious/blob/v6.2.0/src/connection.js#L1655-L1673 Tedious may or may not emit an error, depending on the state the request is currently in. I will need to look at the cancellation test a bit more carefully to handle this. |
- Need to fix context propagation on callbacks - Need to add tests testing instanceName config option, different functions that end up calling makeRequest, canceling Requests, etc.
Thanks Roch!
2e6b5bb
to
c4f46fe
Compare
What does this PR do?
This PR adds an integration for Tedious.
Motivation
We want to automatically trace the Tedious.
Plugin Checklist
Additional Notes
Cancelling requests behaviour changed in v6.2.0: https://github.com/tediousjs/tedious/blob/v6.2.0/src/connection.js#L1655-L1673
Tedious may or may not emit an error, depending on the state the request is currently in.