Skip to content
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

[MongoDB] add TLS CA File steps to docs #24352

Merged
merged 4 commits into from
Jan 14, 2025

Conversation

Apidcloud
Copy link
Contributor

@Apidcloud Apidcloud commented Jan 13, 2025

Description

Noticed there's no documentation on how to really connect Presto to a MongoDB Cluster, as it usually requires access to a TLS Certificate.

Motivation and Context

When I was setting up Presto, I had to add the TLS CA File we have from Digital Ocean to connect to the MongoDB Cluster. I noticed the docs are missing steps for such thing, despite being a very common requirement. This adds that missing information, step by step.

Release Notes

== RELEASE NOTES ==

MongoDB Connector Changes
* Add steps to connect to MongoDB cluster with TLS CA File to :doc:`/connector/mongodb`. :pr:`24352`

@Apidcloud Apidcloud requested review from steveburnett, elharo and a team as code owners January 13, 2025 16:30
@Apidcloud Apidcloud requested a review from presto-oss January 13, 2025 16:30
@github-actions github-actions bot added the docs label Jan 13, 2025
@tdcmeehan tdcmeehan self-assigned this Jan 13, 2025
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

Thank you for the new documentation! I think your steps might be better off with better visibility so I made suggestions of how to move them out of the property definition, and into a new heading in this page that you would link to. Let me know what you think!

presto-docs/src/main/sphinx/connector/mongodb.rst Outdated Show resolved Hide resolved
presto-docs/src/main/sphinx/connector/mongodb.rst Outdated Show resolved Hide resolved
@steveburnett
Copy link
Contributor

Suggest changes to release note entry as follows:

== RELEASE NOTES ==

MongoDB Connector Changes
* Add steps to connect to MongoDB cluster with TLS CA File to :doc:`/connector/mongodb`.  :pr:`24352`

@Apidcloud
Copy link
Contributor Author

Thanks for the review and suggestions. I think it looks a lot better now

Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

Thank you for the quick structural revision! Now the document is better organized, these comments are at the sentence level. Let me know what you think, please.

presto-docs/src/main/sphinx/connector/mongodb.rst Outdated Show resolved Hide resolved
presto-docs/src/main/sphinx/connector/mongodb.rst Outdated Show resolved Hide resolved
presto-docs/src/main/sphinx/connector/mongodb.rst Outdated Show resolved Hide resolved
presto-docs/src/main/sphinx/connector/mongodb.rst Outdated Show resolved Hide resolved
presto-docs/src/main/sphinx/connector/mongodb.rst Outdated Show resolved Hide resolved
presto-docs/src/main/sphinx/connector/mongodb.rst Outdated Show resolved Hide resolved
presto-docs/src/main/sphinx/connector/mongodb.rst Outdated Show resolved Hide resolved
presto-docs/src/main/sphinx/connector/mongodb.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

Thanks! One last thing, didn't notice anything else.

presto-docs/src/main/sphinx/connector/mongodb.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@steveburnett steveburnett left a comment

Choose a reason for hiding this comment

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

LGTM! (docs)

Pull updated branch, new local doc build, looks good. Thanks!

@Apidcloud Apidcloud force-pushed the docs/add-info-about-mongo-tls-ca branch from a9ed390 to fdac83f Compare January 14, 2025 08:37
@Apidcloud Apidcloud changed the title [MongoDB] add TLA CA File steps to docs [MongoDB] add TLS CA File steps to docs Jan 14, 2025
@tdcmeehan
Copy link
Contributor

LGTM. @Apidcloud in the future, please squash commits per our development guidelines. Thanks!

@tdcmeehan tdcmeehan merged commit 761ab56 into prestodb:master Jan 14, 2025
53 checks passed
@Apidcloud
Copy link
Contributor Author

Thanks, @tdcmeehan. I was going to ask whether GitHub supports squashing upon merging, like Gitlab. Thanks for letting me know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants