You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/01/02 11:01:48 UTC
[GitHub] [pulsar] Jennifer88huang opened a new pull request #9108: [WIP][docs]Add topic-level policy config
Jennifer88huang opened a new pull request #9108:
URL: https://github.com/apache/pulsar/pull/9108
### Motivation
Topic-level policies are available in Pulsar 2.7.0.
### Modifications
Add how to configure topic-level policies.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jennifer88huang commented on a change in pull request #9108: [docs]Add topic-level policy config
Posted by GitBox <gi...@apache.org>.
Jennifer88huang commented on a change in pull request #9108:
URL: https://github.com/apache/pulsar/pull/9108#discussion_r554664083
##########
File path: site2/docs/reference-pulsar-admin.md
##########
@@ -1759,13 +1759,20 @@ Options
|`--broker`|Broker name to get namespace-isolation policies attached to it||
## `topics`
-Operations for managing Pulsar topics (both persistent and non persistent)
+Operations for managing Pulsar topics (both persistent and non-persistent).
Usage
```bash
$ pulsar-admin topics subcommand
```
+From Pulsar 2.7.0, some namespace level policies are available on topic level. To enable topic level policy in Pulsar, you need to configure the following parameters in the `broker.conf` file.
Review comment:
Updated, thank you.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jennifer88huang edited a comment on pull request #9108: [docs]Add topic-level policy config
Posted by GitBox <gi...@apache.org>.
Jennifer88huang edited a comment on pull request #9108:
URL: https://github.com/apache/pulsar/pull/9108#issuecomment-757888011
@freeznet Thank you for your comments. Is it better to keep consistent for them in the [code](https://github.com/apache/pulsar/blob/master/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java#L1040-L1048)?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on pull request #9108: [docs]Add topic-level policy config
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #9108:
URL: https://github.com/apache/pulsar/pull/9108#issuecomment-757940664
@freeznet I think `get-maxProducers` is mistaken in the previous PR. We can add `get-max-producers` command first and add `@Deprecated` for the non-lower-case commands. And we can also make the non-lower-case command does not appear while run `bin/pulsar-admin topics`. This will guarantee compatibility.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Huanli-Meng commented on a change in pull request #9108: [docs]Add topic-level policy config
Posted by GitBox <gi...@apache.org>.
Huanli-Meng commented on a change in pull request #9108:
URL: https://github.com/apache/pulsar/pull/9108#discussion_r554651609
##########
File path: site2/docs/reference-pulsar-admin.md
##########
@@ -1759,13 +1759,20 @@ Options
|`--broker`|Broker name to get namespace-isolation policies attached to it||
## `topics`
-Operations for managing Pulsar topics (both persistent and non persistent)
+Operations for managing Pulsar topics (both persistent and non-persistent).
Usage
```bash
$ pulsar-admin topics subcommand
```
+From Pulsar 2.7.0, some namespace level policies are available on topic level. To enable topic level policy in Pulsar, you need to configure the following parameters in the `broker.conf` file.
Review comment:
```suggestion
From Pulsar 2.7.0, some namespace-level policies are available on topic level. To enable topic-level policies in Pulsar, you need to configure the following parameters in the `broker.conf` file.
```
same comments for docs of other releases.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jennifer88huang commented on pull request #9108: [docs]Add topic-level policy config
Posted by GitBox <gi...@apache.org>.
Jennifer88huang commented on pull request #9108:
URL: https://github.com/apache/pulsar/pull/9108#issuecomment-757969447
@freeznet @codelipenghui thank you for your confirmation.
For those in the red boxes, I want to say that in namespaces, we use the word "xxx-per-xxx" (see screenshot below). Since those topic-level polices come from the namespace level, is it more user-friendly if we keep consistent and use the word "per" for topic-level policies? Otherwise, users might use the wrong commands if they want to adopt namespace-level policies to topic-level polices.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] freeznet commented on pull request #9108: [docs]Add topic-level policy config
Posted by GitBox <gi...@apache.org>.
freeznet commented on pull request #9108:
URL: https://github.com/apache/pulsar/pull/9108#issuecomment-757671491
LGTM
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jennifer88huang merged pull request #9108: [docs]Add topic-level policy config
Posted by GitBox <gi...@apache.org>.
Jennifer88huang merged pull request #9108:
URL: https://github.com/apache/pulsar/pull/9108
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on pull request #9108: [docs]Add topic-level policy config
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #9108:
URL: https://github.com/apache/pulsar/pull/9108#issuecomment-757996853
@Jennifer88huang No, topic policy always for one topic, so we don't need to add -per-topic
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] freeznet removed a comment on pull request #9108: [docs]Add topic-level policy config
Posted by GitBox <gi...@apache.org>.
freeznet removed a comment on pull request #9108:
URL: https://github.com/apache/pulsar/pull/9108#issuecomment-757671491
LGTM
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jennifer88huang commented on pull request #9108: [docs]Add topic-level policy config
Posted by GitBox <gi...@apache.org>.
Jennifer88huang commented on pull request #9108:
URL: https://github.com/apache/pulsar/pull/9108#issuecomment-769543781
Thank you @315157973 for fixing the command name issues in #9215. Shall we go on with the doc PR? @freeznet
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jennifer88huang commented on pull request #9108: [docs]Add topic-level policy config
Posted by GitBox <gi...@apache.org>.
Jennifer88huang commented on pull request #9108:
URL: https://github.com/apache/pulsar/pull/9108#issuecomment-757888011
@freeznet Thank you for your comments. Is it better to keep consistent for them in the [code](https://github.com/apache/pulsar/blob/master/pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java#L1040-L1048)?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] freeznet commented on pull request #9108: [docs]Add topic-level policy config
Posted by GitBox <gi...@apache.org>.
freeznet commented on pull request #9108:
URL: https://github.com/apache/pulsar/pull/9108#issuecomment-757936710
@Jennifer88huang I think all commands should in lower case and unified format, as for the blue blocks, like `get-maxProducers`, should changed to `get-max-producers`. For the red blocks, I think they are fine, since the command name is just same as the function name, like `GetMaxConsumersPerSubscription` -> `get-max-consumers-per-subscription`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jennifer88huang commented on pull request #9108: [docs]Add topic-level policy config
Posted by GitBox <gi...@apache.org>.
Jennifer88huang commented on pull request #9108:
URL: https://github.com/apache/pulsar/pull/9108#issuecomment-769709823
Thank you @freeznet
@315157973 @codelipenghui @Huanli-Meng do you have any concern on it?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] freeznet commented on a change in pull request #9108: [docs]Add topic-level policy config
Posted by GitBox <gi...@apache.org>.
freeznet commented on a change in pull request #9108:
URL: https://github.com/apache/pulsar/pull/9108#discussion_r554862932
##########
File path: site2/docs/reference-pulsar-admin.md
##########
@@ -1815,6 +1822,36 @@ Subcommands
* `get-deduplication`
* `set-deduplication`
* `remove-deduplication`
+* `get-max-producers`
+* `set-max-producers`
+* `remove-max-producers`
+* `get-max-consumers`
+* `set-max-consumers`
+* `remove-max-consumers`
+* `get-retention`
+* `set-retention`
+* `remove-retention`
+* `get-dispatch-rate`
+* `set-dispatch-rate`
+* `remove-dispatch-rate`
+* `get-compaction-threshold`
+* `set-compaction-threshold`
+* `remove-compaction-threshold`
+* `get-offload-policies`
+* `set-offload-policies`
+* `remove-offload-policies`
+* `get-max-unacked-messages-per-subscription`
Review comment:
`get-max-unacked-messages-on-subscription`
##########
File path: site2/docs/reference-pulsar-admin.md
##########
@@ -1815,6 +1822,36 @@ Subcommands
* `get-deduplication`
* `set-deduplication`
* `remove-deduplication`
+* `get-max-producers`
+* `set-max-producers`
+* `remove-max-producers`
+* `get-max-consumers`
+* `set-max-consumers`
+* `remove-max-consumers`
+* `get-retention`
+* `set-retention`
+* `remove-retention`
+* `get-dispatch-rate`
+* `set-dispatch-rate`
+* `remove-dispatch-rate`
+* `get-compaction-threshold`
+* `set-compaction-threshold`
+* `remove-compaction-threshold`
+* `get-offload-policies`
+* `set-offload-policies`
+* `remove-offload-policies`
+* `get-max-unacked-messages-per-subscription`
+* `set-max-unacked-messages-per-subscription`
Review comment:
`set-max-unacked-messages-on-subscription`
##########
File path: site2/docs/reference-pulsar-admin.md
##########
@@ -1815,6 +1822,36 @@ Subcommands
* `get-deduplication`
* `set-deduplication`
* `remove-deduplication`
+* `get-max-producers`
+* `set-max-producers`
+* `remove-max-producers`
+* `get-max-consumers`
+* `set-max-consumers`
+* `remove-max-consumers`
+* `get-retention`
+* `set-retention`
+* `remove-retention`
+* `get-dispatch-rate`
+* `set-dispatch-rate`
+* `remove-dispatch-rate`
+* `get-compaction-threshold`
+* `set-compaction-threshold`
+* `remove-compaction-threshold`
+* `get-offload-policies`
+* `set-offload-policies`
+* `remove-offload-policies`
+* `get-max-unacked-messages-per-subscription`
+* `set-max-unacked-messages-per-subscription`
+* `remove-max-unacked-messages-per-subscription`
Review comment:
`remove-max-unacked-messages-on-subscription`
##########
File path: site2/docs/reference-pulsar-admin.md
##########
@@ -1815,6 +1822,36 @@ Subcommands
* `get-deduplication`
* `set-deduplication`
* `remove-deduplication`
+* `get-max-producers`
Review comment:
according to code, this should be `get-maxProducers`, `set-maxProducers` and `remove-maxProducers`
##########
File path: site2/docs/reference-pulsar-admin.md
##########
@@ -1815,6 +1822,36 @@ Subcommands
* `get-deduplication`
* `set-deduplication`
* `remove-deduplication`
+* `get-max-producers`
+* `set-max-producers`
+* `remove-max-producers`
+* `get-max-consumers`
+* `set-max-consumers`
+* `remove-max-consumers`
+* `get-retention`
+* `set-retention`
+* `remove-retention`
+* `get-dispatch-rate`
+* `set-dispatch-rate`
+* `remove-dispatch-rate`
+* `get-compaction-threshold`
+* `set-compaction-threshold`
+* `remove-compaction-threshold`
+* `get-offload-policies`
+* `set-offload-policies`
+* `remove-offload-policies`
+* `get-max-unacked-messages-per-subscription`
+* `set-max-unacked-messages-per-subscription`
+* `remove-max-unacked-messages-per-subscription`
+* `get-max-unacked-messages-per-consumer`
Review comment:
`get-max-unacked-messages-on-consumer`
##########
File path: site2/docs/reference-pulsar-admin.md
##########
@@ -1815,6 +1822,36 @@ Subcommands
* `get-deduplication`
* `set-deduplication`
* `remove-deduplication`
+* `get-max-producers`
+* `set-max-producers`
+* `remove-max-producers`
+* `get-max-consumers`
+* `set-max-consumers`
+* `remove-max-consumers`
+* `get-retention`
+* `set-retention`
+* `remove-retention`
+* `get-dispatch-rate`
+* `set-dispatch-rate`
+* `remove-dispatch-rate`
+* `get-compaction-threshold`
+* `set-compaction-threshold`
+* `remove-compaction-threshold`
+* `get-offload-policies`
+* `set-offload-policies`
+* `remove-offload-policies`
+* `get-max-unacked-messages-per-subscription`
+* `set-max-unacked-messages-per-subscription`
+* `remove-max-unacked-messages-per-subscription`
+* `get-max-unacked-messages-per-consumer`
+* `set-max-unacked-messages-per-consumer`
+* `remove-max-unacked-messages-per-consumer`
Review comment:
`remove-max-unacked-messages-on-consumer`
##########
File path: site2/docs/reference-pulsar-admin.md
##########
@@ -1815,6 +1822,36 @@ Subcommands
* `get-deduplication`
* `set-deduplication`
* `remove-deduplication`
+* `get-max-producers`
+* `set-max-producers`
+* `remove-max-producers`
+* `get-max-consumers`
+* `set-max-consumers`
+* `remove-max-consumers`
+* `get-retention`
+* `set-retention`
+* `remove-retention`
+* `get-dispatch-rate`
+* `set-dispatch-rate`
+* `remove-dispatch-rate`
+* `get-compaction-threshold`
+* `set-compaction-threshold`
+* `remove-compaction-threshold`
+* `get-offload-policies`
+* `set-offload-policies`
+* `remove-offload-policies`
+* `get-max-unacked-messages-per-subscription`
+* `set-max-unacked-messages-per-subscription`
+* `remove-max-unacked-messages-per-subscription`
+* `get-max-unacked-messages-per-consumer`
+* `set-max-unacked-messages-per-consumer`
Review comment:
`set-max-unacked-messages-on-consumer`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jennifer88huang commented on pull request #9108: [docs]Add topic-level policy config
Posted by GitBox <gi...@apache.org>.
Jennifer88huang commented on pull request #9108:
URL: https://github.com/apache/pulsar/pull/9108#issuecomment-758334399
@sijia-w confirmed with eng, we need to keep topic-level commands consistent with that in the namespace-level.
So it should be "get-max-producers", "get-max-unacked-messages-per-consumer", get-max-unacked-messages-per-subscription". Use this kind of format in the docs, and eng will refine the code accordingly.
cc @codelipenghui @freeznet
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jennifer88huang edited a comment on pull request #9108: [docs]Add topic-level policy config
Posted by GitBox <gi...@apache.org>.
Jennifer88huang edited a comment on pull request #9108:
URL: https://github.com/apache/pulsar/pull/9108#issuecomment-757969447
@freeznet @codelipenghui thank you for your confirmation.
For those in the red boxes, I want to say that in namespaces, we use the word "xxx-per-xxx" (see screenshot below). Since those topic-level polices come from the namespace level, is it more user-friendly if we keep consistent and use the word "per" for topic-level policies? Otherwise, users might use the wrong commands if they want to adopt namespace-level policies to topic-level polices. cc @sijia-w
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org