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/04 04:40:43 UTC
[GitHub] [pulsar] poorbarcode opened a new pull request, #15929: [fix] [admin] Enhanced verification for retention policy
poorbarcode opened a new pull request, #15929:
URL: https://github.com/apache/pulsar/pull/15929
### Motivation
Lego ran into an issue where they would call the /admin/v2/persistent/{tenant}/{namespace}/{topic}/retention endpoint after creating a topic, but the call appeared to have no effect, despite returning HTTP 204.
After some initial debugging, they determined that the root cause was related to the casing of the parameters in their JSON payload. They initially provided this:
```
{"RetentionSizeInMb":-1,"RetentionTimeInMinutes":40320}
```
Which should have been this:
```
{"retentionSizeInMB":-1,"retentionTimeInMinutes":40320}
```
This will trick the user into thinking the setup succeeded when it didn't
#### Observed behavior
When an invalid config is submitted, /admin/v2/persistent/{tenant}/{namespace}/{topic}/retention returns HTTP 204
#### Expected behavior
When an invalid config is submitted, /admin/v2/persistent/{tenant}/{namespace}/{topic}/retention returns HTTP 400
### Modifications
Enhanced verification
### Documentation
- [ ] `doc-required`
- [x] `doc-not-needed`
- [ ] `doc`
- [ ] `doc-complete`
--
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] codelipenghui commented on a diff in pull request #15929: [fix] [admin] Enhanced verification for retention policy
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on code in PR #15929:
URL: https://github.com/apache/pulsar/pull/15929#discussion_r889820621
##########
pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/RetentionPolicies.java:
##########
@@ -28,24 +28,33 @@
* Infinite retention can be achieved by setting both time and size limits to `-1`.
*/
public class RetentionPolicies {
- private int retentionTimeInMinutes;
- private long retentionSizeInMB;
+
+ public static final int DEFAULT_RETENTION_TIME_IN_MINUTES = 0;
+
+ public static final long DEFAULT_RETENTION_SIZE_IN_MB = 0;
+
+ private Integer retentionTimeInMinutes;
+ private Long retentionSizeInMB;
Review Comment:
This will introduce a compatibility issue when downgrading to an old version that does not allows
`retentionTimeInMinutes` and `retentionSizeInMB` to zero?
--
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] poorbarcode closed pull request #15929: [fix] [admin] Enhanced verification for retention policy
Posted by GitBox <gi...@apache.org>.
poorbarcode closed pull request #15929: [fix] [admin] Enhanced verification for retention policy
URL: https://github.com/apache/pulsar/pull/15929
--
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] poorbarcode commented on a diff in pull request #15929: [fix] [admin] Enhanced verification for retention policy
Posted by GitBox <gi...@apache.org>.
poorbarcode commented on code in PR #15929:
URL: https://github.com/apache/pulsar/pull/15929#discussion_r889846336
##########
pulsar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/RetentionPolicies.java:
##########
@@ -28,24 +28,33 @@
* Infinite retention can be achieved by setting both time and size limits to `-1`.
*/
public class RetentionPolicies {
- private int retentionTimeInMinutes;
- private long retentionSizeInMB;
+
+ public static final int DEFAULT_RETENTION_TIME_IN_MINUTES = 0;
+
+ public static final long DEFAULT_RETENTION_SIZE_IN_MB = 0;
+
+ private Integer retentionTimeInMinutes;
+ private Long retentionSizeInMB;
Review Comment:
There are no compatibility issues and it can be set to 0. But this PR does not solve the problem that only one property is set correctly, so I will close this 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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org