Skip to content
This repository was archived by the owner on May 1, 2025. It is now read-only.

Conversation

ajhuh-mdb
Copy link
Collaborator

@ajhuh-mdb ajhuh-mdb commented Nov 6, 2024

Copy link

netlify bot commented Nov 6, 2024

Deploy Preview for docs-cluster-to-cluster-sync ready!

Name Link
🔨 Latest commit d4c9731
🔍 Latest deploy log https://app.netlify.com/sites/docs-cluster-to-cluster-sync/deploys/672e394fe6a8b800086f742b
😎 Deploy Preview https://deploy-preview-470--docs-cluster-to-cluster-sync.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.


**Upcoming**


Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note: Embedded verifier content will live here once #339 is merged.

Copy link
Collaborator

@nvillahermosa-mdb nvillahermosa-mdb left a comment

Choose a reason for hiding this comment

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

LGTM

@jtcovan
Copy link
Contributor

jtcovan commented Nov 6, 2024

Thanks Ali, this looks good to me. I just added two more points to the downstream changes doc. Sorry for the late additions:

Under general release notes,

REP-5162: Added a stdin disclaimer to the mongosync executable regarding verification. Users will need to accept this disclaimer before mongosync will run.

REP-5196: Added --acceptDisclaimer flag to bypass the disclaimer for users that have previously read the disclaimer.

These will both be documented in #339 in the relevant sections.

@jtcovan
Copy link
Contributor

jtcovan commented Nov 7, 2024

LGTM, thanks! I'm going to tag one of our leads to do a quick review.


Fixed Issues:

- Fixed a bug introduced in ``mongosync`` 1.8.0 where ``mongosync`` would crash
Copy link
Contributor

Choose a reason for hiding this comment

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

This was already included in the 1.8.1 release notes

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah I don't think we need it (even though it was included in the downstream changes)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Gotcha. I'll remove it!

verification. Users must to accept this disclaimer before ``mongosync``
can run.

- Added ``--acceptDisclaimer`` flag for users that have previously read the
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be documented in the configuration section too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@jtcovan mentioned that this is being documented in #339. If not, would you mind opening up a follow-up ticket, so we can pick that up and address it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Collaborator

@rohshar rohshar left a comment

Choose a reason for hiding this comment

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

What about the note corresponding to:

[REP-5163](https://jira.mongodb.org/browse/REP-5163): Add memory check in /start endpoint for enabling verification. The memory requirement is 0.5 GB per 1 million docs, plus 10 GB of base memory requirement.

@ajhuh-mdb ajhuh-mdb requested review from jddubois and rohshar November 8, 2024 17:43
@ajhuh-mdb ajhuh-mdb merged commit 0592f38 into mongodb:master Nov 11, 2024
4 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants