feat(shell-api): allow disabling "blocking cursor call" warnings MONGOSH-2674 #2531
Conversation
…OSH-2674 Sometimes, users may decide to intentionally use the blocking behavior of these methods. Since they cannot currently disable the warning, we give them a way to do so by calling `cursor.disableBlockWarnings()`. In order to make this a bit easier, we split the existing `AbstractCursor` class into a `BaseCursor` class that contains logic that can be shared with change stream cursors, and a `AbstractFiniteCursor` class that specifically accounts for other existing cursor types. As a side effect, this makes common cursor methods such as `.forEach()` and `.map()` work for change stream cursors.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new method disableBlockWarnings() to allow users to disable blocking cursor call warnings when they intentionally want to use blocking behavior. The implementation refactors the cursor class hierarchy by splitting AbstractCursor into BaseCursor and AbstractFiniteCursor to better support different cursor types.
- Added
disableBlockWarnings()method to cursor classes - Refactored cursor hierarchy to split shared functionality between finite and change stream cursors
- Updated warning messages to mention the new method for disabling warnings
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/shell-api/src/shell-instance-state.ts | Updated currentCursor type to use new BaseCursor base class |
| packages/shell-api/src/shell-api.ts | Added return type annotation for the it() method |
| packages/shell-api/src/run-command-cursor.ts | Updated to extend AbstractFiniteCursor instead of AbstractCursor |
| packages/shell-api/src/helpers.ts | Updated iterate function to work with new cursor hierarchy |
| packages/shell-api/src/cursor.ts | Added warning disable check and updated warning messages |
| packages/shell-api/src/change-stream-cursor.ts | Refactored to extend BaseCursor and implement common cursor methods |
| packages/shell-api/src/change-stream-cursor.spec.ts | Added tests for map() and forEach() functionality on change streams |
| packages/shell-api/src/aggregate-or-find-cursor.ts | Updated to extend AbstractFiniteCursor |
| packages/shell-api/src/abstract-cursor.ts | Split into BaseCursor and AbstractFiniteCursor classes |
| packages/service-provider-core/src/index.ts | Exported ServiceProviderBaseCursor interface |
| packages/service-provider-core/src/cursors.ts | Made ServiceProviderBaseCursor interface public |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
gagik
left a comment
gagik
left a comment
There was a problem hiding this comment.
code looks good.
Do we want this option to be per cursor? My initial thought was a more global config but I'm not really sure what the usecase for disabling this is.
I've also thought about about a global config, but I do feel like this is something that would be specific to things like script usage where you are okay with potentially waiting a long time for a response, and I imagine a script would want to be able to rely on a specific and consistent behavior without affecting the behavior of regular mongosh runs or depending on global config. |
Sometimes, users may decide to intentionally use the blocking behavior of these methods. Since they cannot currently disable the warning, we give them a way to do so by calling
cursor.disableBlockWarnings().In order to make this a bit easier, we split the existing
AbstractCursorclass into aBaseCursorclass that contains logic that can be shared with change stream cursors, and aAbstractFiniteCursorclass that specifically accounts for other existing cursor types.As a side effect, this makes common cursor methods such as
.forEach()and.map()work for change stream cursors.