You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "nodece (via GitHub)" <gi...@apache.org> on 2026/01/05 08:49:11 UTC
[PR] [fix][admin] Fix asyncGetRequest to handle 204 [pulsar]
nodece opened a new pull request, #25124:
URL: https://github.com/apache/pulsar/pull/25124
### Motivation
This pull request improves the handling and testing of asynchronous GET requests in the Pulsar admin client. The main changes ensure that both HTTP 200 (OK) and 204 (No Content) responses are treated as successful, and introduce comprehensive tests to verify correct behavior for various HTTP response scenarios.
### Modifications
**Enhancements to async GET request handling:**
* Modified `BaseResource.java` so that both HTTP 200 (OK) and 204 (No Content) responses are accepted as successful. For 204 responses, the future is completed with `null` since there is no response body to process.
**Testing improvements:**
* Added a new test class `AsyncGetRequestTest.java` to thoroughly test the async GET request logic. The tests cover successful responses with and without a body (200 and 204), as well as error responses like 404 and 500, and 200 with an empty body.
**Testability and code maintenance:**
* Exposed the `root` WebTarget in `PulsarAdminImpl` via a new `@VisibleForTesting` method to facilitate direct testing of request paths.
* Added the necessary import for `@VisibleForTesting` to `PulsarAdminImpl.java`.
### Documentation
<!-- DO NOT REMOVE THIS SECTION. CHECK THE PROPER BOX ONLY. -->
- [ ] `doc` <!-- Your PR contains doc changes. -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->
--
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] [fix][admin] Fix asyncGetRequest to handle 204 [pulsar]
Posted by "nodece (via GitHub)" <gi...@apache.org>.
nodece commented on PR #25124:
URL: https://github.com/apache/pulsar/pull/25124#issuecomment-3709956928
@lhotari I noticed that `sink`, `source`, `function`, and `worker` all depend on this method. #16377 fixed the behavior for `namespace`, `topic`, and related resources, but this method is still used by other admin resources. Therefore, fixing it here is important to ensure consistent behavior across all dependent components.
`org.apache.pulsar.client.admin.internal.BaseResource.FutureCallback` should be deprecated.
`org.apache.pulsar.client.admin.internal.BaseResource#asyncGetRequest(javax.ws.rs.client.WebTarget, java.lang.Class<? extends T>)` and `org.apache.pulsar.client.admin.internal.BaseResource#asyncGetRequest(javax.ws.rs.client.WebTarget, javax.ws.rs.core.GenericType<T>)` can handle most cases.
--
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] [fix][admin] Fix asyncGetRequest to handle 204 [pulsar]
Posted by "nodece (via GitHub)" <gi...@apache.org>.
nodece merged PR #25124:
URL: https://github.com/apache/pulsar/pull/25124
--
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] [fix][admin] Fix asyncGetRequest to handle 204 [pulsar]
Posted by "lhotari (via GitHub)" <gi...@apache.org>.
lhotari commented on PR #25124:
URL: https://github.com/apache/pulsar/pull/25124#issuecomment-3710125747
> @lhotari I noticed that `sink`, `source`, `function`, and `worker` all depend on this method. #16377 fixed the behavior for `namespace`, `topic`, and related resources, but this method is still used by other admin resources. Therefore, fixing it here is important to ensure consistent behavior across all dependent components.
>
> `org.apache.pulsar.client.admin.internal.BaseResource.FutureCallback` should be deprecated.
>
> `org.apache.pulsar.client.admin.internal.BaseResource#asyncGetRequest(javax.ws.rs.client.WebTarget, java.lang.Class<? extends T>)` and `org.apache.pulsar.client.admin.internal.BaseResource#asyncGetRequest(javax.ws.rs.client.WebTarget, javax.ws.rs.core.GenericType<T>)` can handle most cases.
@nodece Just wondering if something is currently broken due to the lack of properly handling 204 return code.
--
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] [fix][admin] Fix asyncGetRequest to handle 204 [pulsar]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #25124:
URL: https://github.com/apache/pulsar/pull/25124#discussion_r2660877413
##########
pulsar-client-admin/src/test/java/org/apache/pulsar/client/admin/internal/AsyncGetRequestTest.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.client.admin.internal;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.get;
+import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
+import static org.assertj.core.api.Assertions.assertThat;
+import com.github.tomakehurst.wiremock.WireMockServer;
+import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
+import java.io.IOException;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.TimeUnit;
+import lombok.Cleanup;
+import org.apache.pulsar.client.admin.PulsarAdmin;
+import org.apache.pulsar.client.admin.PulsarAdminException;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+public class AsyncGetRequestTest {
+
+ WireMockServer server;
+ String adminUrl;
+
+ @BeforeClass(alwaysRun = true)
+ void beforeClass() throws IOException {
+ server = new WireMockServer(WireMockConfiguration.wireMockConfig()
+ .containerThreads(8)
+ .port(0));
+ server.start();
+ adminUrl = "http://localhost:" + server.port();
+ }
+
+ @AfterClass(alwaysRun = true)
+ void afterClass() {
+ if (server != null) {
+ server.stop();
+ }
+ }
+
+ @Test
+ public void testAsyncGetRequest_200OK_WithBody() throws Exception {
+ // Mock successful response with body
+ server.stubFor(get(urlEqualTo("/200-with-body"))
+ .willReturn(aResponse()
+ .withStatus(200)
+ .withHeader("Content-Type", "application/json")
+ .withBody("{\"name\":\"test-namespace\"}")));
+
+ @Cleanup
+ PulsarAdminImpl admin = (PulsarAdminImpl) PulsarAdmin.builder().serviceHttpUrl(adminUrl).build();
+
+ NamespacesImpl namespaces = (NamespacesImpl) admin.namespaces();
+ CompletableFuture<Object> getRequest =
+ namespaces.asyncGetRequest(admin.getRoot().path("/200-with-body"), Object.class);
+
+ assertThat(getRequest).succeedsWithin(3, TimeUnit.SECONDS);
+
+ // Verify the request was made
+ server.verify(getRequestedFor(urlEqualTo("/200-with-body")));
+ }
+
+ @Test
+ public void testAsyncGetRequest_204NoContent() throws Exception {
+ // Mock 204 No Content response
+ server.stubFor(get(urlEqualTo("/204"))
+ .willReturn(aResponse()
+ .withStatus(204)));
+
+ @Cleanup
+ PulsarAdminImpl admin = (PulsarAdminImpl) PulsarAdmin.builder().serviceHttpUrl(adminUrl).build();
+
+ NamespacesImpl namespaces = (NamespacesImpl) admin.namespaces();
+ CompletableFuture<Object> getRequest =
+ namespaces.asyncGetRequest(admin.getRoot().path("/204"), Object.class);
+
+ assertThat(getRequest).succeedsWithin(3, TimeUnit.SECONDS);
+
+ // Verify the request was made
+ server.verify(getRequestedFor(urlEqualTo("/204")));
+ }
+
+ @Test
+ public void testAsyncGetRequest_404NotFound() throws Exception {
+ // Mock 404 Not Found response
+ server.stubFor(get(urlEqualTo("/404"))
+ .willReturn(aResponse()
+ .withStatus(404)
+ .withHeader("Content-Type", "application/json")
+ .withBody("{\"reason\":\"Not found\"}")));
+
+ @Cleanup
+ PulsarAdminImpl admin = (PulsarAdminImpl) PulsarAdmin.builder().serviceHttpUrl(adminUrl).build();
+
+ NamespacesImpl namespaces = (NamespacesImpl) admin.namespaces();
+ CompletableFuture<Object> getRequest =
+ namespaces.asyncGetRequest(admin.getRoot().path("/404"), Object.class);
+
+ assertThat(getRequest)
+ .failsWithin(3, TimeUnit.SECONDS)
Review Comment:
Remove the extra space before 'SECONDS'. It should be 'TimeUnit.SECONDS' without a space.
##########
pulsar-client-admin/src/test/java/org/apache/pulsar/client/admin/internal/AsyncGetRequestTest.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.client.admin.internal;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.get;
+import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
+import static org.assertj.core.api.Assertions.assertThat;
+import com.github.tomakehurst.wiremock.WireMockServer;
+import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
+import java.io.IOException;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.TimeUnit;
+import lombok.Cleanup;
+import org.apache.pulsar.client.admin.PulsarAdmin;
+import org.apache.pulsar.client.admin.PulsarAdminException;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+public class AsyncGetRequestTest {
+
+ WireMockServer server;
+ String adminUrl;
+
+ @BeforeClass(alwaysRun = true)
+ void beforeClass() throws IOException {
+ server = new WireMockServer(WireMockConfiguration.wireMockConfig()
+ .containerThreads(8)
+ .port(0));
+ server.start();
+ adminUrl = "http://localhost:" + server.port();
+ }
+
+ @AfterClass(alwaysRun = true)
+ void afterClass() {
+ if (server != null) {
+ server.stop();
+ }
+ }
+
+ @Test
+ public void testAsyncGetRequest_200OK_WithBody() throws Exception {
+ // Mock successful response with body
+ server.stubFor(get(urlEqualTo("/200-with-body"))
+ .willReturn(aResponse()
+ .withStatus(200)
+ .withHeader("Content-Type", "application/json")
+ .withBody("{\"name\":\"test-namespace\"}")));
+
+ @Cleanup
+ PulsarAdminImpl admin = (PulsarAdminImpl) PulsarAdmin.builder().serviceHttpUrl(adminUrl).build();
+
+ NamespacesImpl namespaces = (NamespacesImpl) admin.namespaces();
+ CompletableFuture<Object> getRequest =
+ namespaces.asyncGetRequest(admin.getRoot().path("/200-with-body"), Object.class);
+
+ assertThat(getRequest).succeedsWithin(3, TimeUnit.SECONDS);
+
+ // Verify the request was made
+ server.verify(getRequestedFor(urlEqualTo("/200-with-body")));
+ }
+
+ @Test
+ public void testAsyncGetRequest_204NoContent() throws Exception {
+ // Mock 204 No Content response
+ server.stubFor(get(urlEqualTo("/204"))
+ .willReturn(aResponse()
+ .withStatus(204)));
+
+ @Cleanup
+ PulsarAdminImpl admin = (PulsarAdminImpl) PulsarAdmin.builder().serviceHttpUrl(adminUrl).build();
+
+ NamespacesImpl namespaces = (NamespacesImpl) admin.namespaces();
+ CompletableFuture<Object> getRequest =
+ namespaces.asyncGetRequest(admin.getRoot().path("/204"), Object.class);
+
+ assertThat(getRequest).succeedsWithin(3, TimeUnit.SECONDS);
+
+ // Verify the request was made
+ server.verify(getRequestedFor(urlEqualTo("/204")));
Review Comment:
Remove the extra space before 'verify'. It should be 'server.verify' without a space.
##########
pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/PulsarAdminImpl.java:
##########
@@ -445,4 +446,9 @@ public void close() {
asyncHttpConnector.close();
}
+
+ @VisibleForTesting
+ WebTarget getRoot() {
Review Comment:
There's an extra leading space before 'WebTarget' on this line. The indentation should be consistent with other methods in the class. Remove the extra space so the line starts with proper indentation.
```suggestion
WebTarget getRoot() {
```
##########
pulsar-client-admin/src/test/java/org/apache/pulsar/client/admin/internal/AsyncGetRequestTest.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.client.admin.internal;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.get;
+import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
+import static org.assertj.core.api.Assertions.assertThat;
+import com.github.tomakehurst.wiremock.WireMockServer;
+import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
+import java.io.IOException;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.TimeUnit;
+import lombok.Cleanup;
+import org.apache.pulsar.client.admin.PulsarAdmin;
+import org.apache.pulsar.client.admin.PulsarAdminException;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+public class AsyncGetRequestTest {
+
+ WireMockServer server;
+ String adminUrl;
+
+ @BeforeClass(alwaysRun = true)
+ void beforeClass() throws IOException {
+ server = new WireMockServer(WireMockConfiguration.wireMockConfig()
+ .containerThreads(8)
+ .port(0));
+ server.start();
+ adminUrl = "http://localhost:" + server.port();
+ }
+
+ @AfterClass(alwaysRun = true)
+ void afterClass() {
+ if (server != null) {
+ server.stop();
+ }
+ }
+
+ @Test
+ public void testAsyncGetRequest_200OK_WithBody() throws Exception {
+ // Mock successful response with body
+ server.stubFor(get(urlEqualTo("/200-with-body"))
+ .willReturn(aResponse()
+ .withStatus(200)
+ .withHeader("Content-Type", "application/json")
+ .withBody("{\"name\":\"test-namespace\"}")));
+
+ @Cleanup
+ PulsarAdminImpl admin = (PulsarAdminImpl) PulsarAdmin.builder().serviceHttpUrl(adminUrl).build();
+
+ NamespacesImpl namespaces = (NamespacesImpl) admin.namespaces();
+ CompletableFuture<Object> getRequest =
+ namespaces.asyncGetRequest(admin.getRoot().path("/200-with-body"), Object.class);
+
+ assertThat(getRequest).succeedsWithin(3, TimeUnit.SECONDS);
+
+ // Verify the request was made
+ server.verify(getRequestedFor(urlEqualTo("/200-with-body")));
+ }
+
+ @Test
+ public void testAsyncGetRequest_204NoContent() throws Exception {
+ // Mock 204 No Content response
+ server.stubFor(get(urlEqualTo("/204"))
+ .willReturn(aResponse()
+ .withStatus(204)));
+
+ @Cleanup
+ PulsarAdminImpl admin = (PulsarAdminImpl) PulsarAdmin.builder().serviceHttpUrl(adminUrl).build();
+
+ NamespacesImpl namespaces = (NamespacesImpl) admin.namespaces();
+ CompletableFuture<Object> getRequest =
+ namespaces.asyncGetRequest(admin.getRoot().path("/204"), Object.class);
+
+ assertThat(getRequest).succeedsWithin(3, TimeUnit.SECONDS);
Review Comment:
The test should verify that the CompletableFuture completes with null for a 204 No Content response. According to the implementation in BaseResource.java, 204 responses complete the future with null since there's no response body. Add an assertion to verify the result is null, for example: 'assertThat(getRequest.get()).isNull()'.
##########
pulsar-client-admin/src/test/java/org/apache/pulsar/client/admin/internal/AsyncGetRequestTest.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.client.admin.internal;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.get;
+import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
+import static org.assertj.core.api.Assertions.assertThat;
+import com.github.tomakehurst.wiremock.WireMockServer;
+import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
+import java.io.IOException;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.TimeUnit;
+import lombok.Cleanup;
+import org.apache.pulsar.client.admin.PulsarAdmin;
+import org.apache.pulsar.client.admin.PulsarAdminException;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+public class AsyncGetRequestTest {
+
+ WireMockServer server;
+ String adminUrl;
+
+ @BeforeClass(alwaysRun = true)
+ void beforeClass() throws IOException {
+ server = new WireMockServer(WireMockConfiguration.wireMockConfig()
+ .containerThreads(8)
+ .port(0));
+ server.start();
+ adminUrl = "http://localhost:" + server.port();
+ }
+
+ @AfterClass(alwaysRun = true)
+ void afterClass() {
+ if (server != null) {
+ server.stop();
+ }
+ }
+
+ @Test
+ public void testAsyncGetRequest_200OK_WithBody() throws Exception {
+ // Mock successful response with body
+ server.stubFor(get(urlEqualTo("/200-with-body"))
+ .willReturn(aResponse()
+ .withStatus(200)
+ .withHeader("Content-Type", "application/json")
+ .withBody("{\"name\":\"test-namespace\"}")));
+
+ @Cleanup
+ PulsarAdminImpl admin = (PulsarAdminImpl) PulsarAdmin.builder().serviceHttpUrl(adminUrl).build();
+
+ NamespacesImpl namespaces = (NamespacesImpl) admin.namespaces();
+ CompletableFuture<Object> getRequest =
+ namespaces.asyncGetRequest(admin.getRoot().path("/200-with-body"), Object.class);
+
+ assertThat(getRequest).succeedsWithin(3, TimeUnit.SECONDS);
Review Comment:
Remove the extra space before 'SECONDS'. It should be 'TimeUnit.SECONDS' without a space.
##########
pulsar-client-admin/src/test/java/org/apache/pulsar/client/admin/internal/AsyncGetRequestTest.java:
##########
@@ -0,0 +1,168 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.client.admin.internal;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.get;
+import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
+import static org.assertj.core.api.Assertions.assertThat;
+import com.github.tomakehurst.wiremock.WireMockServer;
+import com.github.tomakehurst.wiremock.core.WireMockConfiguration;
+import java.io.IOException;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.TimeUnit;
+import lombok.Cleanup;
+import org.apache.pulsar.client.admin.PulsarAdmin;
+import org.apache.pulsar.client.admin.PulsarAdminException;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+public class AsyncGetRequestTest {
+
+ WireMockServer server;
+ String adminUrl;
+
+ @BeforeClass(alwaysRun = true)
+ void beforeClass() throws IOException {
+ server = new WireMockServer(WireMockConfiguration.wireMockConfig()
+ .containerThreads(8)
+ .port(0));
+ server.start();
+ adminUrl = "http://localhost:" + server.port();
+ }
+
+ @AfterClass(alwaysRun = true)
+ void afterClass() {
+ if (server != null) {
+ server.stop();
+ }
+ }
+
+ @Test
+ public void testAsyncGetRequest_200OK_WithBody() throws Exception {
+ // Mock successful response with body
+ server.stubFor(get(urlEqualTo("/200-with-body"))
+ .willReturn(aResponse()
+ .withStatus(200)
+ .withHeader("Content-Type", "application/json")
+ .withBody("{\"name\":\"test-namespace\"}")));
+
+ @Cleanup
+ PulsarAdminImpl admin = (PulsarAdminImpl) PulsarAdmin.builder().serviceHttpUrl(adminUrl).build();
+
+ NamespacesImpl namespaces = (NamespacesImpl) admin.namespaces();
+ CompletableFuture<Object> getRequest =
+ namespaces.asyncGetRequest(admin.getRoot().path("/200-with-body"), Object.class);
+
+ assertThat(getRequest).succeedsWithin(3, TimeUnit.SECONDS);
+
+ // Verify the request was made
+ server.verify(getRequestedFor(urlEqualTo("/200-with-body")));
+ }
+
+ @Test
+ public void testAsyncGetRequest_204NoContent() throws Exception {
+ // Mock 204 No Content response
+ server.stubFor(get(urlEqualTo("/204"))
+ .willReturn(aResponse()
+ .withStatus(204)));
+
+ @Cleanup
+ PulsarAdminImpl admin = (PulsarAdminImpl) PulsarAdmin.builder().serviceHttpUrl(adminUrl).build();
+
+ NamespacesImpl namespaces = (NamespacesImpl) admin.namespaces();
+ CompletableFuture<Object> getRequest =
+ namespaces.asyncGetRequest(admin.getRoot().path("/204"), Object.class);
+
+ assertThat(getRequest).succeedsWithin(3, TimeUnit.SECONDS);
+
+ // Verify the request was made
+ server.verify(getRequestedFor(urlEqualTo("/204")));
+ }
+
+ @Test
+ public void testAsyncGetRequest_404NotFound() throws Exception {
+ // Mock 404 Not Found response
+ server.stubFor(get(urlEqualTo("/404"))
+ .willReturn(aResponse()
+ .withStatus(404)
+ .withHeader("Content-Type", "application/json")
+ .withBody("{\"reason\":\"Not found\"}")));
+
+ @Cleanup
+ PulsarAdminImpl admin = (PulsarAdminImpl) PulsarAdmin.builder().serviceHttpUrl(adminUrl).build();
+
+ NamespacesImpl namespaces = (NamespacesImpl) admin.namespaces();
+ CompletableFuture<Object> getRequest =
+ namespaces.asyncGetRequest(admin.getRoot().path("/404"), Object.class);
Review Comment:
Remove the extra space before 'getRoot()'. It should be 'admin.getRoot()' without a space.
--
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] [fix][admin] Fix asyncGetRequest to handle 204 [pulsar]
Posted by "nodece (via GitHub)" <gi...@apache.org>.
nodece commented on PR #25124:
URL: https://github.com/apache/pulsar/pull/25124#issuecomment-3710197931
I believe most use cases still work today. In my environment, I use the pulsar-admin API to get the replicator dispatch rate of a namespace. This is on a private Pulsar distribution, not upstream Apache Pulsar.
That said, Apache Pulsar itself still relies on this behavior in several admin APIs, so proper handling of HTTP 204 responses is required. This PR ensures consistent and correct behavior in those cases.
--
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] [fix][admin] Fix asyncGetRequest to handle 204 [pulsar]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #25124:
URL: https://github.com/apache/pulsar/pull/25124#issuecomment-3710492366
## [Codecov](https://app.codecov.io/gh/apache/pulsar/pull/25124?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 74.48%. Comparing base ([`f101811`](https://app.codecov.io/gh/apache/pulsar/commit/f10181173357b13b690abb288b83a3a458292e2d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`62ff354`](https://app.codecov.io/gh/apache/pulsar/commit/62ff3547ae04ad285895ae8df36535bf1c252b80?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/pulsar/pull/25124?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #25124 +/- ##
=============================================
+ Coverage 38.75% 74.48% +35.73%
- Complexity 13215 33684 +20469
=============================================
Files 1842 1899 +57
Lines 145545 149716 +4171
Branches 16916 17405 +489
=============================================
+ Hits 56400 111512 +55112
+ Misses 81460 29322 -52138
- Partials 7685 8882 +1197
```
| [Flag](https://app.codecov.io/gh/apache/pulsar/pull/25124/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [inttests](https://app.codecov.io/gh/apache/pulsar/pull/25124/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `26.55% <28.57%> (-0.03%)` | :arrow_down: |
| [systests](https://app.codecov.io/gh/apache/pulsar/pull/25124/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `22.99% <42.85%> (-0.08%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pulsar/pull/25124/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `74.01% <100.00%> (+39.46%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Files with missing lines](https://app.codecov.io/gh/apache/pulsar/pull/25124?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...che/pulsar/client/admin/internal/BaseResource.java](https://app.codecov.io/gh/apache/pulsar/pull/25124?src=pr&el=tree&filepath=pulsar-client-admin%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fclient%2Fadmin%2Finternal%2FBaseResource.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWNsaWVudC1hZG1pbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9hZG1pbi9pbnRlcm5hbC9CYXNlUmVzb3VyY2UuamF2YQ==) | `78.82% <100.00%> (+24.60%)` | :arrow_up: |
| [.../pulsar/client/admin/internal/PulsarAdminImpl.java](https://app.codecov.io/gh/apache/pulsar/pull/25124?src=pr&el=tree&filepath=pulsar-client-admin%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpulsar%2Fclient%2Fadmin%2Finternal%2FPulsarAdminImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHVsc2FyLWNsaWVudC1hZG1pbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcHVsc2FyL2NsaWVudC9hZG1pbi9pbnRlcm5hbC9QdWxzYXJBZG1pbkltcGwuamF2YQ==) | `79.24% <100.00%> (+10.67%)` | :arrow_up: |
... and [1404 files with indirect coverage changes](https://app.codecov.io/gh/apache/pulsar/pull/25124/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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