DRIVERS-1975: Add showExpandedEvents flag and new change stream events
#1220
Conversation
baileympearson
changed the title
showExpandedEvents flag and new change stream events
| | "refineCollectionShardKey" | ||
| | "reshardCollection"; | ||
|
|
There was a problem hiding this comment.
refineCollectionShardKey and reshardCollection events are behind a (currently unsupported) feature flag. I included them here for completeness, but notice that there are no spec tests testing for these new events. If there are no objections, I can file a follow up ticket to add tests for these events once they're available.
There was a problem hiding this comment.
Gotcha. Sounds like the feature flag won't be supported until 6.1.0. I agree that we should file a follow-up ticket and add tests once the feature flag can be enabled and drivers can actually test.
source/change-streams/tests/unified/change-streams-showExpandedEvents.yml
Show resolved
Hide resolved
source/change-streams/tests/unified/change-streams-showExpandedEvents.yml
Show resolved
Hide resolved
source/change-streams/tests/unified/change-streams-showExpandedEvents.yml
Show resolved
Hide resolved
source/change-streams/tests/unified/change-streams-showExpandedEvents.yml
Show resolved
Hide resolved
- remove unused entities - add test for 'create' on a view
| @@ -208,7 +229,27 @@ If an aggregate command with a ``$changeStream`` stage completes successfully, t | |||
| * The wall time from the mongod that the change event originated from. | |||
| * Populated for server versions 6.0 and above. | |||
| */ | |||
| wallTime: Optional<Datetime>; | |||
| wallTime: DateTime; | |||
There was a problem hiding this comment.
Why was this changed to non-optional? AFAIK this isn't populated <6.0.
There was a problem hiding this comment.
You're right, that was unintentional. I changed it back to optional.
| */ | ||
| updateDescription: Optional<UpdateDescription>; | ||
| updateDescription: Optional<UpdateDescription | Document>; |
There was a problem hiding this comment.
This is a breaking change for the Rust driver (going from a struct to an enum in a public API); from the syntax doc, it looks like the underlying change is the addition of a specialFields field?
There was a problem hiding this comment.
Actually this change was unintentional. Originally I wrote the spec update to include the showRawUpdateDescription and showSystemEvents flags but these flags are internal and not intended to be exposed so drivers don't have to support them.
The specialFields and rawUpdateDescription fields on the UpdateDescription are guarded behind showRawUpdateDescription, and so don't need to be added.
| arguments: | ||
| pipeline: [] | ||
| showExpandedEvents: true | ||
| showSystemEvents: true |
There was a problem hiding this comment.
Since showSystemEvents isn't mentioned in the spec yet, maybe this test should wait for a follow-up?
There was a problem hiding this comment.
Related to my comment above about showSystemEvents and showRawUpdateDescription - this change is unnecessary. Thanks for catching this.
Unlike other languages, Node loads the spec tests directly into JS objects (basically just dictionaries) so we don't run into serialization errors like other languages might if extra, unsupported options are present!
This PR adds support for a new change stream flag,
showExpandedEvents. This flag enables a set of new change stream events and new fields on existing change stream documents.Technical Design: https://docs.google.com/document/d/1BWElA2_AUH5Wupjnr1igYMZflQTRmuszoNh2T6NAqwY/edit#
Syntax Update Document: https://docs.google.com/document/d/1DHlvXSbrLVR_jnnf96AbUqoyakL_oIp9KEI4Gw3R6II/edit#
Node Driver POC - mongodb/node-mongodb-native#3254
Please complete the following before merging: