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

Conversation

@kanchana-mongodb
Copy link
Collaborator

@kanchana-mongodb kanchana-mongodb commented Feb 29, 2024

Tested and updated doc for Atlas connection string fix. This required fixes in multiple pages.

{8:12}~/server-bug-bash ➭ mongosync \
      --logPath data/log/mongosync \
      --cluster0 "mongodb+srv://testUser:testPwd@c2ctestcluster1.w2z3k.mongodb-dev.net/" \
      --cluster1 "mongodb+srv://testUser:testPwd@c2ctestcluster2.w2z3k.mongodb-dev.net/" 

{9:19}~/server-bug-bash ➭ curl localhost:27182/api/v1/start -XPOST \
--data '
   {
      "source": "cluster0",
      "destination": "cluster1"
   } '

{"success":true}%  

@kanchana-mongodb kanchana-mongodb force-pushed the DOCS-15748 branch 2 times, most recently from 339cc87 to cebcff3 Compare February 29, 2024 18:38
Copy link
Collaborator

@kennethdyer kennethdyer left a comment

Choose a reason for hiding this comment

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

Thanks for this work @kanchana-mongodb, I left a few comments for your consideration.

The generic connection string format for the Atlas cluster is:

.. code-block:: shell
Copy link
Collaborator

Choose a reason for hiding this comment

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

For this code block, set the format to text since it's a raw example and doesn't need syntax highlighting.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm still seeing these set to shell. They should be text or none.

Use the connection information you gathered for the self-managed cluster
to create the connection strings for ``cluster0``:

.. code-block:: shell
Copy link
Collaborator

Choose a reason for hiding this comment

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

This block doesn't provide a shell example of the --cluster0 option. Instead it's showing an example of the YAML configuration file, so it should use the yaml syntax highlighter.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not seeing the change here, either.

Copy link

@AleGonez AleGonez left a comment

Choose a reason for hiding this comment

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

Requested changes seems to be OK. However, I’m seeing some repeated information in Connect a Self-Managed Cluster to Atlas stage link. Following information is twice:

image

Can you please remove the duplicated info?
I'm not seeing this was part of the changes for this PR though but it would be good to correct it before publishing. Let me know if this will be handled in a separate PR and I can approve this one.

@kanchana-mongodb
Copy link
Collaborator Author

Requested changes seems to be OK. However, I’m seeing some repeated information in Connect a Self-Managed Cluster to Atlas stage link. Following information is twice:

image

Can you please remove the duplicated info? I'm not seeing this was part of the changes for this PR though but it would be good to correct it before publishing. Let me know if this will be handled in a separate PR and I can approve this one.

Actually, the connection string format is different - the first description shows the format for self-managed clusters followed by format for Atlas clusters. Clarified this in the page. Thanks!

Copy link
Collaborator

@kennethdyer kennethdyer left a comment

Choose a reason for hiding this comment

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

Thanks @kanchana-mongodb, Didn't see the syntax highlighting changes but those are non-blocking suggestions. LGTM!

@jmd-mongo jmd-mongo merged commit 8da1841 into mongodb:master Mar 6, 2024
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.

4 participants