-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[fix](doc): incorrect tls config in java client doc #16315
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
Conversation
Signed-off-by: ericsyh <ericshenyuhao@outlook.com>
Signed-off-by: ericsyh <ericshenyuhao@outlook.com>
|
I think that this looks correct, but there are conflicts to resolve |
Signed-off-by: ericsyh <ericshenyuhao@outlook.com>
Signed-off-by: ericsyh <ericshenyuhao@outlook.com>
@michaeljmarshall @dave2wave Conflicts resolved. PTAL again. |
|
cc @Anonymitaet Could help take a look at this Doc PR or assign a technical review? |
Anonymitaet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gaoran10
could you please review this PR from the technical perspective? Thank you!
site2/docs/client-libraries-java.md
Outdated
| `numListenerThreads`|int|The number of threads used for handling message listeners. The listener thread pool is shared across all the consumers and readers using the "listener" model to get messages. For a given consumer, the listener is always invoked from the same thread to ensure ordering. If you want multiple threads to process a single topic, you need to create a [`shared`](/concepts-messaging.md#shared) subscription and multiple consumers for this subscription. This does not ensure ordering.| 1 | ||
| `useTcpNoDelay`| boolean| Whether to use TCP no-delay flag on the connection to disable Nagle algorithm |true | ||
| `useTls` |boolean |Whether to use TLS encryption on the connection| false | ||
| `enableTls` |boolean | Whether to use TLS encryption on the connection. **Deprecated**. use "pulsar+ssl://" in serviceUrl to enable | false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does Deprecated mean? enableTls is deprecated or something else?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no need to config enableTls in pulsar client to enable Tls, just use pulsar+ssl:// in the connection address
| ### TLS Authentication | ||
|
|
||
| To use [TLS](security-tls-authentication), you need to set TLS to `true` using the `setUseTls` method, point your Pulsar client to a TLS cert path, and provide paths to cert and key files. | ||
| To use [TLS](security-tls-authentication.md), you need to set TLS to `true` using the `enableTls` method, point your Pulsar client to a TLS cert path, and provide paths to cert and key files. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@momo-jun FYI: here is a missing occurrence (the .md issue)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the reminder. I just covered three versions 2.10.0, 2.10.1, and master, but didn't have a chance to fix them in doc versions earlier than 2.10. There are hundreds of them in each version, so I need to implement it by phase to avoid getting dizzy and crazy. Maybe I can file an good-first-issue :)
Signed-off-by: ericsyh <ericshenyuhao@outlook.com>
Signed-off-by: ericsyh <ericshenyuhao@outlook.com>
Co-authored-by: Anonymitaet <50226895+Anonymitaet@users.noreply.github.com>
…a-client-tls-config
Signed-off-by: ericsyh <ericshenyuhao@outlook.com>
tisonkun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution! LGTM.
@ericsyh may you tidy the PR description?
@Anonymitaet it seems this PR needs one more approval from committers to merge, could you help with reviewing?
(If this PR fixes a github issue, please add
Fixes #<xyz>.)Fixes #
(or if this PR is one task of a github issue, please add
Master Issue: #<xyz>to link to the master issue.)Master Issue: #
Motivation
On the java client loadconf page there is a config
useTlson the list but I think this config should be theenableTlsand I also check on ClientBuilder there is no method asuseTlsModifications
Replace the incorrect tls config
useTlsbyenableTlson the java client doc.Verifying this change
(Please pick either of the following options)
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
Does this pull request potentially affect one of the following parts:
If
yeswas chosen, please highlight the changesDocumentation
Check the box below or label this PR directly.
Need to update docs?
doc-required(Your PR needs to update docs and you will update later)
doc-not-needed(Please explain why)
doc(Your PR contains doc changes)
doc-complete(Docs have been already added)