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

DOCSP-33426: socks5 proxy server #146

Merged
merged 3 commits into from
Jan 17, 2024

Conversation

rustagir
Copy link
Collaborator

Pull Request Info

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-33426
Staging:

Self-Review Checklist

  • Is this free of any warnings or errors in the RST?
  • Did you run a spell-check?
  • Did you run a grammar-check?
  • Are all the links working?
  • Are the facets and meta keywords accurate?

Copy link
Collaborator

@jordan-smith721 jordan-smith721 left a comment

Choose a reason for hiding this comment

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

LGTM w/ just a couple suggestions

Comment on lines 58 to 59
- Specifies the TCP port number of the SOCKS5 proxy server. This option
defaults to ``1080`` when you set ``proxyHost``.
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 a bit confused by the last sentence here. It says it defaults to 1080 "when you set proxyHost" but from the description of proxyHost it seems like proxyHost is required? So does that mean this will always default to 1080?
I think my confusion is that this sentence also implies that there is another default, but doesn't specify it. Could you add some clarification here, or possibly remove the "when you set..." part if that still makes sense

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think the way it works is that if you set proxyHost but do not set proxyPort, proxyPort defaults to 1080. However, you can set proxyPort manually to whatever you want. If you do not set proxyHost, then there is no default for proxyPort and it also remains unset. I can clear up that wording.

Comment on lines 89 to 90
using the ``applyToSocketSettings()`` builder method when creating a
``MongoClientSettings``:
Copy link
Collaborator

Choose a reason for hiding this comment

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

S/nit: I think "creating a mongoclientsettings" sounds like incorrect grammar or an incomplete sentence, even though it's technically fine since that's just the name of the instance being created

Suggested change
using the ``applyToSocketSettings()`` builder method when creating a
``MongoClientSettings``:
using the ``applyToSocketSettings()`` builder method when creating a
``MongoClientSettings`` instance:

@rustagir rustagir merged commit f9ec693 into mongodb:master Jan 17, 2024
5 checks passed
@docs-builder-bot
Copy link
Collaborator

rustagir added a commit that referenced this pull request Jan 17, 2024
* DOCSP-33426: socks5 proxy server

* first pass fixes

* JS PR fixes

(cherry picked from commit f9ec693)
ccho-mongodb pushed a commit to ccho-mongodb/docs-kotlin that referenced this pull request Feb 14, 2024
* DOCSP-33426: socks5 proxy server

* first pass fixes

* JS PR fixes
ccho-mongodb pushed a commit to ccho-mongodb/docs-kotlin that referenced this pull request Feb 14, 2024
* DOCSP-33426: socks5 proxy server

* first pass fixes

* JS PR fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants