You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "pgier (via GitHub)" <gi...@apache.org> on 2026/04/10 00:04:44 UTC
[PR] [improve][broker] update broker error message to include tenant name [pulsar]
pgier opened a new pull request, #25503:
URL: https://github.com/apache/pulsar/pull/25503
### Motivation
This is just to improve the log message to include the tenant name when there the tenant is not found
### Modifications
Just updated the log
### Verifying this change
- [X] Make sure that the change passes the CI checks.
This change is a trivial rework / code cleanup without any test coverage.
### Does this pull request potentially affect one of the following parts:
No, it does not affect any of these.
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
*If the box was checked, please highlight the changes*
- [ ] Dependencies (add or upgrade a dependency)
- [ ] The public API
- [ ] The schema
- [ ] The default values of configurations
- [ ] The threading model
- [ ] The binary protocol
- [ ] The REST endpoints
- [ ] The admin CLI options
- [ ] The metrics
- [ ] Anything that affects deployment
--
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
Re: [PR] [improve][broker] update broker error message to include tenant name [pulsar]
Posted by "merlimat (via GitHub)" <gi...@apache.org>.
merlimat commented on code in PR #25503:
URL: https://github.com/apache/pulsar/pull/25503#discussion_r3061437961
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -142,7 +142,7 @@ protected CompletableFuture<List<String>> internalGetTenantNamespaces(String ten
.thenCompose(__ -> tenantResources().tenantExistsAsync(tenant))
.thenCompose(existed -> {
if (!existed) {
- throw new RestException(Status.NOT_FOUND, "Tenant not found");
+ throw new RestException(Status.NOT_FOUND, "Tenant not found: " + tenant);
Review Comment:
The tenant name is already included in the path of the request. I don't think it needs to be in the response too.
--
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
Re: [PR] [improve][broker] update broker error message to include tenant name [pulsar]
Posted by "pgier (via GitHub)" <gi...@apache.org>.
pgier commented on code in PR #25503:
URL: https://github.com/apache/pulsar/pull/25503#discussion_r3065106539
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -142,7 +142,7 @@ protected CompletableFuture<List<String>> internalGetTenantNamespaces(String ten
.thenCompose(__ -> tenantResources().tenantExistsAsync(tenant))
.thenCompose(existed -> {
if (!existed) {
- throw new RestException(Status.NOT_FOUND, "Tenant not found");
+ throw new RestException(Status.NOT_FOUND, "Tenant not found: " + tenant);
Review Comment:
The issue is that the log on the server doesn't show which tenant has the issue. I've updated the code so that it only prints the tenant name in the server log, WDYT?
--
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
Re: [PR] [improve][broker] update broker error message to include tenant name [pulsar]
Posted by "merlimat (via GitHub)" <gi...@apache.org>.
merlimat commented on code in PR #25503:
URL: https://github.com/apache/pulsar/pull/25503#discussion_r3066440545
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/NamespacesBase.java:
##########
@@ -142,7 +142,7 @@ protected CompletableFuture<List<String>> internalGetTenantNamespaces(String ten
.thenCompose(__ -> tenantResources().tenantExistsAsync(tenant))
.thenCompose(existed -> {
if (!existed) {
- throw new RestException(Status.NOT_FOUND, "Tenant not found");
+ throw new RestException(Status.NOT_FOUND, "Tenant not found: " + tenant);
Review Comment:
👍 This makes sense.
My main concern for including in the response was that we have so many of these return error messages, and we should be consistent in what we return :)
--
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
Re: [PR] [improve][broker] update broker error message to include tenant name [pulsar]
Posted by "merlimat (via GitHub)" <gi...@apache.org>.
merlimat merged PR #25503:
URL: https://github.com/apache/pulsar/pull/25503
--
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