-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[fix][doc] correct configuration name to exposingBrokerEntryMetadataToClientEnabled #17099
Conversation
@@ -68,7 +68,7 @@ If you want to use broker entry metadata for **consumers**: | |||
|
|||
1. Use the client protocol version [18 or later](https://github.com/apache/pulsar/blob/ca37e67211feda4f7e0984e6414e707f1c1dfd07/pulsar-common/src/main/proto/PulsarApi.proto#L259). | |||
|
|||
2. Configure the [`brokerEntryMetadataInterceptors`](reference-configuration.md#broker) parameter and set the [`enableExposingBrokerEntryMetadataToClient`](reference-configuration.md#broker) parameter to `true` in the `broker.conf` file. |
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.
The brokerEntryMetadataInterceptors should not appear before V2.10.0. It's the same with #17004
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.
brokerEntryMetadataInterceptors
was introduced since 2.8.0, but enableExposingBrokerEntryMetadataToClient
was introduced since 2.10.0.
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.
LGTM
@BewareMyPower |
ping @BewareMyPower again :-D |
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.
Could you check this comment?
@RobertIndie @BewareMyPower thanks for your suggestions! I've updated this PR based on your comments. The changes are summarized below, PTAL.
cc @momo-jun |
I found another problem when I reviewed this PR but it's not related. I found some versioned docs contain both 2c2
< id: develop-binary-protocol
---
> id: developing-binary-protocol
5c5
< original_id: develop-binary-protocol
---
> original_id: developing-binary-protocol
400a401,425
> * `properties` → *(optional)* Reserved configuration items
> * `txnid_most_bits` → *(optional)* Same as Transaction Coordinator ID, `txnid_most_bits` and `txnid_least_bits`
> uniquely identify a transaction.
> * `txnid_least_bits` → *(optional)* The ID of the transaction opened in a transaction coordinator,
> `txnid_most_bits` and `txnid_least_bits`uniquely identify a transaction.
> * `request_id` → *(optional)* ID for handling response and timeout.
>
>
> ##### Command AckResponse
>
> An `AckResponse` is the broker’s response to acknowledge a request sent by the client. It contains the `consumer_id` sent in the request.
> If a transaction is used, it contains both the Transaction ID and the Request ID that are sent in the request. The client finishes the specific request according to the Request ID. If the `error` field is set, it indicates that the request has failed.
>
> An example of `AckResponse` with redirection:
>
> ```protobuf
>
> message CommandAckResponse {
> "consumer_id" : 1,
> "txnid_least_bits" = 0,
> "txnid_most_bits" = 1,
> "request_id" = 5
> }
>
> ```
461c486
< Lookup can be done with a REST call as described in the [admin API](admin-api-topics.md#look-up-topics-owner-broker)
---
> Lookup can be done with a REST call as described in the [admin API](admin-api-topics.md#lookup-of-topic) Could we delete the develop-binary-protocol.md? |
@BewareMyPower thank you! Suggest keeping it since there might be potential issues if it's removed. Besides, keeping it does not cause trouble for doc maintainers since there is only |
Fix #13336 (review)
I do not fix occurrences in
reference-configuration.md
because we're optimizing the workflow (trying to generate docs from code automatically for https://pulsar.apache.org/docs/next/reference-configuration)doc