AnonHxy commented on code in PR #17238:
URL: https://github.com/apache/pulsar/pull/17238#discussion_r953642381
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java:
##########
@@ -601,6 +602,60 @@ private CompletableFuture<Map<String, String>>
getPropertiesAsync() {
});
}
+ protected CompletableFuture<Void> internalUpdatePropertiesAsync(boolean
authoritative,
+
Map<String, String> properties) {
+ if (properties == null || properties.isEmpty()) {
+ log.warn("[{}] [{}] properties is empty, ignore update",
clientAppId(), topicName);
+ return CompletableFuture.completedFuture(null);
Review Comment:
Users can set the value as null to make the key invalidte. Here we don't
delete the key because I think it's better to keep same action as what
non-partitioned topic do, see below:
https://github.com/apache/pulsar/blob/05a2ea87371783000f496ce8103c0ac372e4a8fe/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java#L3912-L3923
And if we have to delete the key, I think it's better add another API like
`deleteProperty` :) @Jason918
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java:
##########
@@ -966,6 +966,42 @@ public void getProperties(
});
}
+ @PUT
+ @Path("/{tenant}/{namespace}/{topic}/properties")
+ @ApiOperation(value = "Update the properties on the given topic.")
+ @ApiResponses(value = {
+ @ApiResponse(code = 307, message = "Current broker doesn't serve the
namespace of this topic"),
+ @ApiResponse(code = 401, message = "Don't have permission to
administrate resources on this tenant or"
+ + "subscriber is not authorized to access this operation"),
+ @ApiResponse(code = 403, message = "Don't have admin permission"),
+ @ApiResponse(code = 404, message = "Topic/Subscription does not
exist"),
+ @ApiResponse(code = 405, message = "Method Not Allowed"),
+ @ApiResponse(code = 500, message = "Internal server error"),
+ @ApiResponse(code = 503, message = "Failed to validate global cluster
configuration")
+ })
+ public void updateProperties(
+ @Suspended final AsyncResponse asyncResponse,
+ @ApiParam(value = "Specify the tenant", required = true)
+ @PathParam("tenant") String tenant,
+ @ApiParam(value = "Specify the namespace", required = true)
+ @PathParam("namespace") String namespace,
+ @ApiParam(value = "Specify topic name", required = true)
+ @PathParam("topic") @Encoded String encodedTopic,
+ @ApiParam(value = "Whether leader broker redirected this call to this
broker. For internal use.")
+ @QueryParam("authoritative") @DefaultValue("false") boolean
authoritative,
+ @ApiParam(value = "Key value pair properties for the topic metadata")
Map<String, String> properties){
+ validatePersistentTopicName(tenant, namespace, encodedTopic);
Review Comment:
I think that the web service framework should catch all exceptions throw by
API methods, but I am not sure.
##########
pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/Topics.java:
##########
@@ -736,6 +736,24 @@ void updatePartitionedTopic(String topic, int
numPartitions, boolean updateLocal
*/
CompletableFuture<Map<String, String>> getPropertiesAsync(String topic);
+ /**
+ * Update Topic Properties on a topic.
+ * The new properties will override the existing values, properties that
are not passed will be keep.
+ * @param topic
Review Comment:
Updated. PTAL @Jason918
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java:
##########
@@ -18,6 +18,7 @@
*/
package org.apache.pulsar.broker.admin.impl;
+import static org.apache.commons.collections4.MapUtils.putAll;
Review Comment:
OK
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
