-
Notifications
You must be signed in to change notification settings - Fork 79
MONGOSH-376 - Include metadata about data source in result #361
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
I’ve checked that this does make the necessary information available on the VSCode side Also, the naming of some of the things in here could probably be improved, feel free to make suggestions :) |
packages/shell-api/src/decorators.ts
Outdated
if (typeof result === 'object' && result !== null) { | ||
const resultSourceInformation: ShellResultSourceInformation = { | ||
dataSource: obj._dataSourceIdentifier(), | ||
call: functionName, |
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'd just go with the source
(db
and collection
) unless we already have a specific use for call
and arguments
. They shouldn't be necessary for this task (at least for what i've got).
Nothing major but there is a good chance we will have to leave around stuff nobody uses.
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.
Yeah, I’ve gone back and forth on this a bit myself … it feels like something that could be really useful, e.g. for figuring out how to get the data that this call resulted in a second time or displaying what call resulted in this output. I’d lean towards keeping it since it’s essentially free and data that is always going to be available anyway, but I’m also happy to remove it
* @returns {Cursor} The promise of the cursor. | ||
*/ | ||
@returnsPromise | ||
@returnType('Document') |
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.
Nice stuff!
4243e10
to
8cc2d2f
Compare
For results from calls on collections, add metadata about the source of the returned data (database name, collection name, method, arguments) so that consumers of the shell can figure out from what source the result was generated, e.g. in order to understand how to modify that original data. This also adds a fake `Document` type to the `ShellResult` reported for e.g. `findOne()`, so that it is possible to tell those apart from other plain JavaScript objects, e.g. in order to be able to know whether the result is a modifiable object in the database or not.
type: 'AggregationCursor', | ||
value: this._mongo._serviceProvider.platform === ReplPlatform.JavaShell ? this : await this._asPrintable() | ||
value: this._mongo._serviceProvider.platform === ReplPlatform.JavaShell ? this : await this._asPrintable(), | ||
source: this[resultSource] ?? undefined |
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.
Oh cool, I've never seen '??' before.
For results from calls on collections, add metadata about the source of
the returned data (database name, collection name, method, arguments)
so that consumers of the shell can figure out from what source
the result was generated, e.g. in order to understand how to modify
that original data.
This also adds a fake
Document
type to theShellResult
reportedfor e.g.
findOne()
, so that it is possible to tell those apartfrom other plain JavaScript objects, e.g. in order to be able to
know whether the result is a modifiable object in the database or not.