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 2022/06/18 22:15:26 UTC
[GitHub] [pulsar] mans2singh opened a new pull request, #16130: [improvement][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
mans2singh opened a new pull request, #16130:
URL: https://github.com/apache/pulsar/pull/16130
Fixes #16082
### Motivation
* The help option for set retention indicates `-t` option is `Retention time in minutes` but if no time unit is specified, it is assumed to be seconds.
### Modifications
* Updated description for time option and added unit test cases with default value (without time unit suffix) and with seconds time unit.
### Verifying this change
- [x] Make sure that the change passes the CI checks.
This change added tests and can be verified as follows:
- Added unit tests for to `PulsarAdminToolTests with and without time unit suffix.
### Does this pull request potentially affect one of the following parts:
*If `yes` was chosen, please highlight the changes*
- Dependencies (does it add or upgrade a dependency): no
- The public API: no
- The schema: no
- The default values of configurations: no
- The wire protocol: no
- The rest endpoints: no
- The admin cli options: no
- Anything that affects deployment: no
### Documentation
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)
- [x] `doc-not-needed`
(Please explain why)
- [ ] `doc`
(Your PR contains doc changes)
- [ ] `doc-complete`
(Docs have been already added)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mans2singh commented on a diff in pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
mans2singh commented on code in PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#discussion_r928322708
##########
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopicPolicies.java:
##########
@@ -466,11 +466,18 @@ private class SetRetention extends CliCommand {
private List<String> params;
@Parameter(names = { "--time",
- "-t" }, description = "Retention time in minutes (or minutes, hours,days,weeks eg: 100m, 3h, 2d, 5w). "
- + "0 means no retention and -1 means infinite time retention", required = true)
+ "-t" }, description = "Retention time with optional time unit suffix "
+ + "eg: 100m, 3h, 2d, 5w. "
+ + "The suffix character can be s for seconds, m for minutes, h for hours, "
Review Comment:
Updated based on recommendations. Thanks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mans2singh commented on pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
mans2singh commented on PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#issuecomment-1238767606
/pulsarbot run-failure-checks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet merged pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
Anonymitaet merged PR #16130:
URL: https://github.com/apache/pulsar/pull/16130
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] horizonzy commented on pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
horizonzy commented on PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#issuecomment-1195070414
/pulsarbot run-failure-checks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#issuecomment-1237574370
@mans2singh your PR can not be merged since some required tests failed. Can you get all required tests passed?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] horizonzy commented on a diff in pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
horizonzy commented on code in PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#discussion_r928061795
##########
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopicPolicies.java:
##########
@@ -466,11 +466,18 @@ private class SetRetention extends CliCommand {
private List<String> params;
@Parameter(names = { "--time",
- "-t" }, description = "Retention time in minutes (or minutes, hours,days,weeks eg: 100m, 3h, 2d, 5w). "
- + "0 means no retention and -1 means infinite time retention", required = true)
+ "-t" }, description = "Retention time with optional time unit suffix "
+ + "eg: 100m, 3h, 2d, 5w. "
+ + "The suffix character can be s for seconds, m for minutes, h for hours, "
Review Comment:
Still in `CmdTopicPolicies`
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mans2singh commented on a diff in pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
mans2singh commented on code in PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#discussion_r927717238
##########
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java:
##########
@@ -1812,11 +1812,18 @@ private class SetRetention extends CliCommand {
private java.util.List<String> params;
@Parameter(names = { "--time",
- "-t" }, description = "Retention time in minutes (or minutes, hours,days,weeks eg: 100m, 3h, 2d, 5w). "
- + "0 means no retention and -1 means infinite time retention", required = true)
+ "-t" }, description = "Retention time with optional time unit suffix "
+ + "eg: 100m, 3h, 2d, 5w. "
+ + "The suffix character can be s for seconds, m for minutes, h for hours, "
+ + "d for days, w for weeks or y for years. "
+ + "If no time unit is specified the default is seconds, eg: "
+ + "-t 120 will set retention to 2 minutes. "
+ + "0 means no retention and -1 means infinite time retention.", required = true)
private String retentionTimeStr;
- @Parameter(names = { "--size", "-s" }, description = "Retention size limit (eg: 10M, 16G, 3T). "
+ @Parameter(names = { "--size", "-s" }, description = "Retention size limit with optional size unit suffix "
+ + "eg: 4096, 10M, 16G, 3T. The size unit suffix character can be k/K, m/M, g/G, or t/G. "
Review Comment:
Thanks for catching my error. Corrected.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on a diff in pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on code in PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#discussion_r932802359
##########
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopicPolicies.java:
##########
@@ -466,11 +466,16 @@ private class SetRetention extends CliCommand {
private List<String> params;
@Parameter(names = { "--time",
- "-t" }, description = "Retention time in minutes (or minutes, hours,days,weeks eg: 100m, 3h, 2d, 5w). "
- + "0 means no retention and -1 means infinite time retention", required = true)
+ "-t" }, description = "Retention time with optional time unit suffix "
+ + "eg: 100m, 3h, 2d, 5w. "
Review Comment:
```suggestion
+ "For example, 100m, 3h, 2d, 5w. "
```
##########
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopicPolicies.java:
##########
@@ -466,11 +466,16 @@ private class SetRetention extends CliCommand {
private List<String> params;
@Parameter(names = { "--time",
- "-t" }, description = "Retention time in minutes (or minutes, hours,days,weeks eg: 100m, 3h, 2d, 5w). "
- + "0 means no retention and -1 means infinite time retention", required = true)
+ "-t" }, description = "Retention time with optional time unit suffix "
+ + "eg: 100m, 3h, 2d, 5w. "
+ + "If no time unit is specified the default is seconds, eg: "
Review Comment:
```suggestion
+ "If the time unit is not specified, the default unit is second. For example, "
```
##########
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopicPolicies.java:
##########
@@ -466,11 +466,16 @@ private class SetRetention extends CliCommand {
private List<String> params;
@Parameter(names = { "--time",
- "-t" }, description = "Retention time in minutes (or minutes, hours,days,weeks eg: 100m, 3h, 2d, 5w). "
- + "0 means no retention and -1 means infinite time retention", required = true)
+ "-t" }, description = "Retention time with optional time unit suffix "
+ + "eg: 100m, 3h, 2d, 5w. "
+ + "If no time unit is specified the default is seconds, eg: "
+ + "-t 120 will set retention to 2 minutes. "
Review Comment:
```suggestion
+ "-t 120 sets retention to 2 minutes. "
```
Write in the simple present tense as much as possible if you are covering facts that were, are, and forever shall be true.
https://docs.google.com/document/d/1lc5j4RtuLIzlEYCBo97AC8-U_3Erzs_lxpkDuseU0n4/edit?pli=1#bookmark=id.e8uqh1awkcnp
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mans2singh commented on pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
mans2singh commented on PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#issuecomment-1169966382
/pulsarbot run-failure-checks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mans2singh commented on a diff in pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
mans2singh commented on code in PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#discussion_r927716905
##########
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdNamespaces.java:
##########
@@ -741,8 +741,13 @@ private class SetRetention extends CliCommand {
private java.util.List<String> params;
@Parameter(names = { "--time",
- "-t" }, description = "Retention time in minutes (or minutes, hours,days,weeks eg: 100m, 3h, 2d, 5w). "
- + "0 means no retention and -1 means infinite time retention", required = true)
+ "-t" }, description = "Retention time with optional time unit suffix "
Review Comment:
I missed that one, I've updated 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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mans2singh commented on pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
mans2singh commented on PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#issuecomment-1239375555
Thanks @Anonymitaet @horizonzy for your help.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] horizonzy commented on pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
horizonzy commented on PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#issuecomment-1189822163
> @Anonymitaet, @horizonzy - Please let me know if you have any feedback on this PR. Thanks
Fine.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] horizonzy commented on pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
horizonzy commented on PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#issuecomment-1198006792
@Anonymitaet ping
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] horizonzy commented on a diff in pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
horizonzy commented on code in PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#discussion_r927574297
##########
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdNamespaces.java:
##########
@@ -741,8 +741,13 @@ private class SetRetention extends CliCommand {
private java.util.List<String> params;
@Parameter(names = { "--time",
- "-t" }, description = "Retention time in minutes (or minutes, hours,days,weeks eg: 100m, 3h, 2d, 5w). "
- + "0 means no retention and -1 means infinite time retention", required = true)
+ "-t" }, description = "Retention time with optional time unit suffix "
Review Comment:
This file retention size doc miss update.
##########
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopics.java:
##########
@@ -1812,11 +1812,18 @@ private class SetRetention extends CliCommand {
private java.util.List<String> params;
@Parameter(names = { "--time",
- "-t" }, description = "Retention time in minutes (or minutes, hours,days,weeks eg: 100m, 3h, 2d, 5w). "
- + "0 means no retention and -1 means infinite time retention", required = true)
+ "-t" }, description = "Retention time with optional time unit suffix "
+ + "eg: 100m, 3h, 2d, 5w. "
+ + "The suffix character can be s for seconds, m for minutes, h for hours, "
+ + "d for days, w for weeks or y for years. "
+ + "If no time unit is specified the default is seconds, eg: "
+ + "-t 120 will set retention to 2 minutes. "
+ + "0 means no retention and -1 means infinite time retention.", required = true)
private String retentionTimeStr;
- @Parameter(names = { "--size", "-s" }, description = "Retention size limit (eg: 10M, 16G, 3T). "
+ @Parameter(names = { "--size", "-s" }, description = "Retention size limit with optional size unit suffix "
+ + "eg: 4096, 10M, 16G, 3T. The size unit suffix character can be k/K, m/M, g/G, or t/G. "
Review Comment:
t/G -> t/T
##########
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopicPolicies.java:
##########
@@ -466,11 +466,18 @@ private class SetRetention extends CliCommand {
private List<String> params;
@Parameter(names = { "--time",
- "-t" }, description = "Retention time in minutes (or minutes, hours,days,weeks eg: 100m, 3h, 2d, 5w). "
- + "0 means no retention and -1 means infinite time retention", required = true)
+ "-t" }, description = "Retention time with optional time unit suffix "
+ + "eg: 100m, 3h, 2d, 5w. "
+ + "The suffix character can be s for seconds, m for minutes, h for hours, "
Review Comment:
> "The suffix character can be s for seconds, m for minutes, h for hours, d for days, w for weeks or y for years. "
I think it is not necessary.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mans2singh commented on pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
mans2singh commented on PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#issuecomment-1192642132
@horizonzy - Thanks for your thorough feedback. I've updated the code based on your recommendations.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mans2singh commented on pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
mans2singh commented on PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#issuecomment-1238757217
/pulsarbot run-failure-checks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mans2singh commented on pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
mans2singh commented on PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#issuecomment-1201871556
@Anonymitaet -
I have updated the description based on your recommendations. Please let me know if there is anything else required.
Thanks again for your advice/time.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mans2singh commented on pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
mans2singh commented on PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#issuecomment-1170638942
@Anonymitaet -
Can you please review this PR regarding issue (https://github.com/apache/pulsar/issues/16082) ? It clarifies that the default time unit (without time unit) is seconds. I have added supporting unit test cases with and without time unit.
From my understanding the failing unit tests (TEST-org.apache.pulsar.broker.service.RackAwareTest.xml) is not related this PR or it's tests.
Please let me know if there is anything else required.
Thanks for your time.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mans2singh commented on pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
mans2singh commented on PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#issuecomment-1234028998
@Anonymitaet @horizonzy - Please let me know if there is anything required for this PR to be merged to master. Thanks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#issuecomment-1234900448
/pulsarbot run-failure-checks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mans2singh commented on pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
mans2singh commented on PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#issuecomment-1235522197
/pulsarbot run-failure-checks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mans2singh commented on a diff in pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
mans2singh commented on code in PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#discussion_r934965062
##########
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopicPolicies.java:
##########
@@ -466,11 +466,16 @@ private class SetRetention extends CliCommand {
private List<String> params;
@Parameter(names = { "--time",
- "-t" }, description = "Retention time in minutes (or minutes, hours,days,weeks eg: 100m, 3h, 2d, 5w). "
- + "0 means no retention and -1 means infinite time retention", required = true)
+ "-t" }, description = "Retention time with optional time unit suffix "
+ + "eg: 100m, 3h, 2d, 5w. "
+ + "If no time unit is specified the default is seconds, eg: "
Review Comment:
Updated.
##########
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopicPolicies.java:
##########
@@ -466,11 +466,16 @@ private class SetRetention extends CliCommand {
private List<String> params;
@Parameter(names = { "--time",
- "-t" }, description = "Retention time in minutes (or minutes, hours,days,weeks eg: 100m, 3h, 2d, 5w). "
- + "0 means no retention and -1 means infinite time retention", required = true)
+ "-t" }, description = "Retention time with optional time unit suffix "
+ + "eg: 100m, 3h, 2d, 5w. "
+ + "If no time unit is specified the default is seconds, eg: "
+ + "-t 120 will set retention to 2 minutes. "
Review Comment:
Updated. Thanks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#issuecomment-1203395827
Thanks for your contribution! LGTM from the technical writing perspective 😊
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mans2singh commented on a diff in pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
mans2singh commented on code in PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#discussion_r934964961
##########
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopicPolicies.java:
##########
@@ -466,11 +466,16 @@ private class SetRetention extends CliCommand {
private List<String> params;
@Parameter(names = { "--time",
- "-t" }, description = "Retention time in minutes (or minutes, hours,days,weeks eg: 100m, 3h, 2d, 5w). "
- + "0 means no retention and -1 means infinite time retention", required = true)
+ "-t" }, description = "Retention time with optional time unit suffix "
+ + "eg: 100m, 3h, 2d, 5w. "
Review Comment:
Sounds good. Thanks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mans2singh commented on pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
mans2singh commented on PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#issuecomment-1192086976
Hi @horizonzy -
Thanks for your feedback.
I've updated description for size parameter and added test cases as you had recommended.
Please let me know if you have any additional advice.
Thanks again for your time.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mans2singh commented on pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
mans2singh commented on PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#issuecomment-1238751713
/pulsarbot run-failure-checks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mans2singh commented on pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
mans2singh commented on PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#issuecomment-1159839575
/pulsarbot run-failure-checks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#issuecomment-1170650544
@horizonzy
could you please review this PR from the technical perspective? 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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mans2singh commented on pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
mans2singh commented on PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#issuecomment-1189696161
@Anonymitaet, @horizonzy - Please let me know if you have any feedback on this PR. Thanks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mans2singh commented on a diff in pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
mans2singh commented on code in PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#discussion_r927717546
##########
pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdTopicPolicies.java:
##########
@@ -466,11 +466,18 @@ private class SetRetention extends CliCommand {
private List<String> params;
@Parameter(names = { "--time",
- "-t" }, description = "Retention time in minutes (or minutes, hours,days,weeks eg: 100m, 3h, 2d, 5w). "
- + "0 means no retention and -1 means infinite time retention", required = true)
+ "-t" }, description = "Retention time with optional time unit suffix "
+ + "eg: 100m, 3h, 2d, 5w. "
+ + "The suffix character can be s for seconds, m for minutes, h for hours, "
Review Comment:
Removed the extra line from description.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mans2singh commented on pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
mans2singh commented on PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#issuecomment-1192462851
/pulsarbot run-failure-checks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mans2singh commented on pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
mans2singh commented on PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#issuecomment-1238786500
Hi @Anonymitaet -
All checks are passing now.
Please let me know if there is anything else required.
Thanks again for your guidance.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mans2singh commented on pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
mans2singh commented on PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#issuecomment-1237534927
@Anonymitaet @horizonzy - The failing tests (Pulsar CI/CI - Unit Broker - Flaky, Pulsar CI/ CI - System Pulsar Connectors, CI - CPP, Python Tests / System-PULSAR_CONNECTORS_THREAD, CI - CPP, Python Tests / Unit-BROKER_FLAKY Tests) are not related this PR and are still failing despite invoking `pulsarbot run-failure-checks`.
Please let me know if there is anything required from my side to merge this to master.
Thanks again for your time and advice.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mans2singh commented on pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
mans2singh commented on PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#issuecomment-1170366796
/pulsarbot run-failure-checks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] mans2singh commented on pull request #16130: [improve][pulsar-client-tools][ISSUE-16082] - Updated set retention time description and added test cases including default time
Posted by GitBox <gi...@apache.org>.
mans2singh commented on PR #16130:
URL: https://github.com/apache/pulsar/pull/16130#issuecomment-1205937699
@Anonymitaet - It looks like PR is pending your review feedback (`1 pending reviewer` above). Please let me know if there is anything I can address. Thanks again for all your advice.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org