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 2020/10/23 09:43:27 UTC
[GitHub] [pulsar] lhotari opened a new pull request #8358: [Issue 8346][Admin API] Validate retention policy
lhotari opened a new pull request #8358:
URL: https://github.com/apache/pulsar/pull/8358
Fixes #8346
### Motivation
See #8346, #8345
### Modifications
Prevent setting invalid retention policy where either size or time limit is set to zero while the other limit has a non-zero value.
The reason for this is that setting either size or time limit to zero will effectively disable the retention policy.
It is confusing for the end-user if it's possible to set a value for the other limit while it's ignored when the other limit has
the value of zero. This might lead to the incorrect assumption that the value of 0 has the meaning that the limit isn't effective.
The validation will provide the Admin API end user a useful error message if the validation fails.
----------------------------------------------------------------
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 #8358: [Issue 8346][Admin API] Validate retention policy
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #8358:
URL: https://github.com/apache/pulsar/pull/8358#issuecomment-715932777
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #8358: [Issue 8346][Admin API] Validate retention policy
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #8358:
URL: https://github.com/apache/pulsar/pull/8358#issuecomment-715342600
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #8358: [Issue 8346][Admin API] Validate retention policy
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #8358:
URL: https://github.com/apache/pulsar/pull/8358#issuecomment-715409332
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #8358: [Issue 8346][Admin API] Validate retention policy
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #8358:
URL: https://github.com/apache/pulsar/pull/8358#issuecomment-715487192
I'm running an experiment to see if reverting 647d3c2 fixes the grpc / protobuf compatibility issues.
----------------------------------------------------------------
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] lhotari commented on pull request #8358: [Issue 8346][Admin API] Validate retention policy
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #8358:
URL: https://github.com/apache/pulsar/pull/8358#issuecomment-715300760
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie commented on a change in pull request #8358: [Issue 8346][Admin API] Validate retention policy
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #8358:
URL: https://github.com/apache/pulsar/pull/8358#discussion_r510974409
##########
File path: pom.xml
##########
@@ -126,7 +126,7 @@ flexible messaging model and an intuitive client API.</description>
<protobuf3.version>3.11.4</protobuf3.version>
<protoc3.version>3.11.4</protoc3.version>
<grpc.version>1.31.0</grpc.version>
- <protoc-gen-grpc-java.version>1.12.0</protoc-gen-grpc-java.version>
+ <protoc-gen-grpc-java.version>${grpc.version}</protoc-gen-grpc-java.version>
Review comment:
Is this change related? If not, I'd suggest removing this change from this pull request. Especially we can the way how we define the dependency to using `grpc-bom`. It should be done in a separate PR.
----------------------------------------------------------------
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] lhotari commented on pull request #8358: [Issue 8346][Admin API] Validate retention policy
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #8358:
URL: https://github.com/apache/pulsar/pull/8358#issuecomment-715255264
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #8358: [Issue 8346][Admin API] Validate retention policy
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #8358:
URL: https://github.com/apache/pulsar/pull/8358#issuecomment-715459464
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #8358: [Issue 8346][Admin API] Validate retention policy
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #8358:
URL: https://github.com/apache/pulsar/pull/8358#issuecomment-716132736
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #8358: [Issue 8346][Admin API] Validate retention policy
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #8358:
URL: https://github.com/apache/pulsar/pull/8358#issuecomment-716189616
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #8358: [Issue 8346][Admin API] Validate retention policy
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #8358:
URL: https://github.com/apache/pulsar/pull/8358#issuecomment-716139286
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #8358: [Issue 8346][Admin API] Validate retention policy
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #8358:
URL: https://github.com/apache/pulsar/pull/8358#issuecomment-716197124
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #8358: [Issue 8346][Admin API] Validate retention policy
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #8358:
URL: https://github.com/apache/pulsar/pull/8358#issuecomment-716058670
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #8358: [Issue 8346][Admin API] Validate retention policy
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #8358:
URL: https://github.com/apache/pulsar/pull/8358#issuecomment-715384296
since master branch is broken atm, I cherry picked the commit from #8361 to fix the broken state of CI. That PR should be merged before this one.
----------------------------------------------------------------
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] lhotari commented on a change in pull request #8358: [Issue 8346][Admin API] Validate retention policy
Posted by GitBox <gi...@apache.org>.
lhotari commented on a change in pull request #8358:
URL: https://github.com/apache/pulsar/pull/8358#discussion_r511058050
##########
File path: pom.xml
##########
@@ -126,7 +126,7 @@ flexible messaging model and an intuitive client API.</description>
<protobuf3.version>3.11.4</protobuf3.version>
<protoc3.version>3.11.4</protoc3.version>
<grpc.version>1.31.0</grpc.version>
- <protoc-gen-grpc-java.version>1.12.0</protoc-gen-grpc-java.version>
+ <protoc-gen-grpc-java.version>${grpc.version}</protoc-gen-grpc-java.version>
Review comment:
yes, this is completely unrelated. I was running the experiment in this PR to see if I could fix the grpc upgrade. I'll remove the changes once #8363 has been merged.
----------------------------------------------------------------
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] lhotari commented on pull request #8358: [Issue 8346][Admin API] Validate retention policy
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #8358:
URL: https://github.com/apache/pulsar/pull/8358#issuecomment-716042682
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui merged pull request #8358: [Issue 8346][Admin API] Validate retention policy
Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #8358:
URL: https://github.com/apache/pulsar/pull/8358
----------------------------------------------------------------
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] lhotari commented on pull request #8358: [Issue 8346][Admin API] Validate retention policy
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #8358:
URL: https://github.com/apache/pulsar/pull/8358#issuecomment-715321562
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #8358: [Issue 8346][Admin API] Validate retention policy
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #8358:
URL: https://github.com/apache/pulsar/pull/8358#issuecomment-716048702
/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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org