michaeljmarshall commented on code in PR #18194:
URL: https://github.com/apache/pulsar/pull/18194#discussion_r1005236961
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/NonPersistentTopics.java:
##########
@@ -121,7 +121,9 @@ public void getInternalStats(
@Path("/{property}/{cluster}/{namespace}/{topic}/partitions")
@ApiOperation(hidden = true, value = "Create a partitioned topic.",
notes = "It needs to be called before creating a producer on a
partitioned topic.")
- @ApiResponses(value = {@ApiResponse(code = 403, message = "Don't have
admin permission"),
+ @ApiResponses(value = {
+ @ApiResponse(code = 403, message = "Don't have admin permission"),
+ @ApiResponse(code = 404, message = "Namespace doesn't exist"),
Review Comment:
Nit:
```suggestion
@ApiResponse(code = 404, message = "Namespace or topic does not
exist"),
```
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v1/Namespaces.java:
##########
@@ -881,6 +881,7 @@ public void unloadNamespace(@Suspended final AsyncResponse
asyncResponse, @PathP
@ApiOperation(hidden = true, value = "Unload a namespace bundle")
@ApiResponses(value = {
@ApiResponse(code = 307, message = "Current broker doesn't serve
the namespace"),
+ @ApiResponse(code = 404, message = "Namespace doesn't exist"),
Review Comment:
Nit: I think we probably want all of these to be like this:
```suggestion
@ApiResponse(code = 404, message = "Namespace does not exist"),
```
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/NonPersistentTopics.java:
##########
@@ -541,7 +541,7 @@ public void getEntryFilters(@Suspended AsyncResponse
asyncResponse,
@Path("/{tenant}/{namespace}/{topic}/entryFilters")
@ApiOperation(value = "Set entry filters for specified topic")
@ApiResponses(value = {@ApiResponse(code = 403, message = "Don't have
admin permission"),
- @ApiResponse(code = 404, message = "Topic does not exist"),
+ @ApiResponse(code = 404, message = "Tenant or namespace or topic
doesn't exist"),
Review Comment:
Is there a reason this string has `Tenant`, but most of the others do not?
--
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]
