Skip to content

Conversation

@dariakp
Copy link
Contributor

@dariakp dariakp commented Jul 28, 2021

Description

NODE-2843

What changed?

Implemented the advanceClusterTime session method, refactored internal helper function for naming consistency

@dariakp dariakp added the wip label Jul 28, 2021
@dariakp dariakp added Primary Review In Review with primary reviewer, not yet ready for team's eyes and removed wip labels Jul 30, 2021
Copy link
Contributor

@nbbeeken nbbeeken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dariakp dariakp marked this pull request as ready for review July 30, 2021 22:15
@dariakp dariakp added Team Review Needs review from team and removed Primary Review In Review with primary reviewer, not yet ready for team's eyes labels Jul 30, 2021
@dariakp dariakp requested review from durran and emadum July 30, 2021 22:16
!clusterTime.signature ||
clusterTime.signature.hash?._bsontype !== 'Binary' ||
(typeof clusterTime.signature.keyId !== 'number' &&
clusterTime.signature.keyId?._bsontype !== 'Long') // apparently we decode the key to number?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nbbeeken Just bringing your attention to this change

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just so I do not block this PR: looks correct to me.

Remind me to tell you the deeper issue here / file a ticket next week, has to do with BSON promote* options I think.

@nbbeeken nbbeeken merged commit 1fd0244 into 4.0 Aug 2, 2021
@nbbeeken nbbeeken deleted the NODE-2843/advance_cluster_time branch August 2, 2021 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Team Review Needs review from team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants