-
Notifications
You must be signed in to change notification settings - Fork 1.8k
refactor(NODE-5379): cursor internals to use async-await #3804
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
Conversation
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.
Did we test the changes iin this PR and see if they affect the memory leak reported in NODE-5502?
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.
Did we test the changes iin this PR and see if they affect the memory leak reported in NODE-5502?
No, I was hoping you could do that when you were back we ended up needing more context on that in triage anyway, maybe this branch's changes will reveal something further.
39e7757
to
177a4fc
Compare
66526ee
to
cb4bb3c
Compare
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.
Orca Security Scan Summary
Status | Check | Issues by priority | |
---|---|---|---|
![]() |
Infrastructure as Code | ![]() ![]() ![]() ![]() |
View in Orca |
![]() |
Vulnerabilities | ![]() ![]() ![]() ![]() |
View in Orca |
![]() |
Secrets | ![]() ![]() ![]() ![]() |
View in Orca |
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.
I see we've pulled in the explain test skip PR, but are still failing some explain tests. Is that expected here?
de5f961
to
fce1a86
Compare
@@ -12,7 +12,7 @@ import { | |||
|
|||
const explain = [true, false, 'queryPlanner', 'allPlansExecution', 'executionStats', 'invalid']; | |||
|
|||
describe('CRUD API explain option', function () { | |||
describe.only('CRUD API explain option', function () { |
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.
Remove .only
Description
refactored cursor internals to use async-await syntax instead of callbacks
What is changing?
getMore, _initialize, cleanupCursor, and [kInit] now use async-await syntax
all classes that inherit from AbstractCursor now use async-await
Is there new documentation needed for these changes?
None
What is the motivation for this change?
further converting the driver to async-await
Double check the following
npm run check:lint
scripttype(NODE-xxxx)[!]: description
feat(NODE-1234)!: rewriting everything in coffeescript