You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "bharathbikkavolu (via GitHub)" <gi...@apache.org> on 2026/09/28 04:54:21 UTC
[PR] CASSANDRA-21195: Extract reusable HTTP utility from cloud metadata connector [cassandra]
bharathbikkavolu opened a new pull request, #5219:
URL: https://github.com/apache/cassandra/pull/5219
## Summary
Extracts the HTTP transport logic from `AbstractCloudMetadataServiceConnector` into a reusable `HttpUtil` utility under `org.apache.cassandra.utils`.
The refactoring separates HTTP transport from caller-specific response handling while preserving the existing behavior of `AbstractCloudMetadataServiceConnector`.
**JIRA:** [CASSANDRA-21195](https://issues.apache.org/jira/browse/CASSANDRA-21195)
## Motivation
`AbstractCloudMetadataServiceConnector` currently contains HTTP transport logic directly, including connection setup, request headers, timeouts, and response-body handling.
This functionality can also be useful to other Cassandra components that need to perform synchronous HTTP operations during startup or initialization. For example, future integrations with secret-management systems such as Vault may need to make HTTP requests to retrieve secrets or authentication tokens.
Providing a shared HTTP utility allows these components to reuse the transport implementation instead of implementing their own HTTP connection handling.
`HttpUtil` is intentionally limited to HTTP transport. Response validation, parsing, and interpretation remain the responsibility of the caller.
## Changes
### `HttpUtil`
Adds `org.apache.cassandra.utils.HttpUtil` with:
- `HttpConfig` for connect timeout, read timeout, and request headers.
- `HttpResponse` containing the raw HTTP status code, status message, and response body.
- `executeGet()` as a convenience method for GET requests.
- `execute()` for executing requests with a specified HTTP method.
- Content-length-based response handling consistent with the existing connector behavior.
- `null` response body when `Content-Length` is unknown (`-1`).
- Error-stream handling for `4xx`/`5xx` responses so the HTTP response can be returned to the caller for validation.
- Connection cleanup after each request.
### `AbstractCloudMetadataServiceConnector`
Refactors `apiCall()` to delegate HTTP transport to `HttpUtil.execute()`.
Existing behavior is preserved:
- Expected response-code validation remains in `AbstractCloudMetadataServiceConnector`.
- The existing connector-specific `HttpException` is retained.
- The configured metadata request timeout continues to be used as the connection timeout.
- The read timeout remains `0`, preserving the previous behavior where no explicit read timeout was configured.
No caller-specific response parsing or validation has been moved into `HttpUtil`.
## Tests
Adds `HttpUtilTest` using an embedded `com.sun.net.httpserver.HttpServer`.
Coverage includes:
- GET requests
- PUT requests
- Custom request headers
- Raw HTTP status and status-message handling
- Read timeout
- Null headers
- Unknown/chunked content length
- Invalid HTTP methods
- `4xx` response-body handling through the error stream
Existing cloud metadata and snitch tests were also run unchanged:
- `Ec2V2ConnectorMockingTest`
- `AzureConnectorMockingTest`
- `DefaultCloudMetadataServiceConnectorTest`
- `Ec2SnitchTest`
- `GoogleCloudSnitchTest`
- `AlibabaCloudSnitchTest`
## Compatibility
This is intended to be a refactoring of the existing cloud metadata HTTP transport behavior.
`AbstractCloudMetadataServiceConnector` retains ownership of expected response-code validation and its existing `HttpException` contract. No configuration changes or user-facing API changes are introduced.
Patch by Bharath Kumar Bikkavolu(bharath.bikkavolu@gmail.com); reviewed by TBD for CASSANDRA-21195
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21195: Extract reusable HTTP utility from cloud metadata connector [cassandra]
Posted by "bharathbikkavolu (via GitHub)" <gi...@apache.org>.
bharathbikkavolu commented on PR #5219:
URL: https://github.com/apache/cassandra/pull/5219#issuecomment-5893729811
@smiklosovic One thing I noticed after the refactor is that the common apiCall() preserves the existing cloud metadata behavior by using a read timeout of 0. Since HttpService can now be extended by other services, do you think we should continue preserving this behavior for this refactoring and let future consumers configure their read timeout as needed, or should HttpService expose a configurable read timeout as part of this change?
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21195: Extract reusable HTTP utility from cloud metadata connector [cassandra]
Posted by "bharathbikkavolu (via GitHub)" <gi...@apache.org>.
bharathbikkavolu commented on code in PR #5219:
URL: https://github.com/apache/cassandra/pull/5219#discussion_r4129419010
##########
src/java/org/apache/cassandra/locator/AbstractCloudMetadataServiceConnector.java:
##########
@@ -103,35 +100,16 @@ public String apiCall(String url,
Map<String, String> extraHeaders,
int expectedResponseCode) throws IOException
{
- HttpURLConnection conn = null;
Review Comment:
Thanks, @smiklosovic . Makes sense. I’ll review the additional extraction in your PR and incorporate the changes.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21195: Extract reusable HTTP utility from cloud metadata connector [cassandra]
Posted by "bharathbikkavolu (via GitHub)" <gi...@apache.org>.
bharathbikkavolu commented on code in PR #5219:
URL: https://github.com/apache/cassandra/pull/5219#discussion_r4130007689
##########
src/java/org/apache/cassandra/locator/AbstractCloudMetadataServiceConnector.java:
##########
Review Comment:
Thanks, @smiklosovic. I reviewed your changes and incorporated the HttpService refactor from your PR. The common API methods and HttpException now live in HttpService, while the raw execute() methods remain available for callers that need to handle the response directly.
I also reran the relevant tests with Java 11, Java 17, and Java 21, and they are passing.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21195: Extract reusable HTTP utility from cloud metadata connector [cassandra]
Posted by "bharathbikkavolu (via GitHub)" <gi...@apache.org>.
bharathbikkavolu commented on code in PR #5219:
URL: https://github.com/apache/cassandra/pull/5219#discussion_r4129923041
##########
src/java/org/apache/cassandra/utils/HttpUtil.java:
##########
@@ -0,0 +1,284 @@
+/*
+ * 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.cassandra.utils;
+
+import java.io.DataInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.util.Map;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+/**
+ * Utility for making synchronous HTTP requests during Cassandra startup and initialization.
+ * This class provides reusable HTTP transport logic for components that need to retrieve
+ * information from external HTTP services during startup.
+ *
+ * <p>This utility is intended for startup-time operations where synchronous, blocking HTTP
+ * requests are appropriate. Examples include retrieving cloud metadata or configuration
+ * information required before Cassandra begins serving requests.
+ *
+ * <p><b>Design principles:</b>
+ * <ul>
+ * <li><b>Synchronous requests only</b> - Requests block until the operation completes or times out</li>
+ * <li><b>Raw HTTP responses</b> - Status-code validation and response parsing are the caller's responsibility</li>
+ * <li><b>Configurable timeouts</b> - Connection and read timeouts are supplied by the caller</li>
+ * <li><b>Header support</b> - Request headers can be supplied by the caller</li>
+ * <li><b>Diagnostic logging</b> - Request metadata is logged without logging header values or response contents</li>
+ * </ul>
+ *
+ * <p>This utility extracts common HTTP transport logic from components such as
+ * {@link org.apache.cassandra.locator.AbstractCloudMetadataServiceConnector} while preserving
+ * caller-specific response handling.
+ */
+public class HttpUtil
+{
+ private static final Logger logger = LoggerFactory.getLogger(HttpUtil.class);
+
+ /**
+ * Configuration for an HTTP request.
+ */
+ public static class HttpConfig
+ {
+ private final int connectTimeoutMs;
+ private final int readTimeoutMs;
+ private final Map<String, String> headers;
+
+ public HttpConfig(int connectTimeoutMs, int readTimeoutMs, Map<String, String> headers)
+ {
+ this.connectTimeoutMs = connectTimeoutMs;
+ this.readTimeoutMs = readTimeoutMs;
+ this.headers = headers != null ? headers : Map.of();
+ }
+
+ public int getConnectTimeoutMs()
+ {
+ return connectTimeoutMs;
+ }
+
+ public int getReadTimeoutMs()
+ {
+ return readTimeoutMs;
+ }
+
+ public Map<String, String> getHeaders()
+ {
+ return headers;
+ }
+ }
+
+ /**
+ * Raw HTTP response containing the status code, status message, and response body.
+ * Interpretation of the response is the caller's responsibility.
+ */
+ public static class HttpResponse
+ {
+ private final int statusCode;
+ private final String statusMessage;
+ private final String body;
+
+ public HttpResponse(int statusCode, String statusMessage, String body)
+ {
+ this.statusCode = statusCode;
+ this.statusMessage = statusMessage;
+ this.body = body;
+ }
+
+ public int getStatusCode()
+ {
+ return statusCode;
+ }
+
+ public String getStatusMessage()
+ {
+ return statusMessage;
+ }
+
+ public String getBody()
+ {
+ return body;
+ }
+ }
+
+ /**
+ * Execute a synchronous HTTP GET request.
+ *
+ * @param url the URL to request
+ * @param config the HTTP configuration containing timeouts and headers
+ * @return the raw HTTP response
+ * @throws IOException if the HTTP request or response read fails
+ */
+ public static HttpResponse executeGet(String url, HttpConfig config) throws IOException
+ {
+ logger.trace("Executing GET request via executeGet convenience method");
+ return execute(url, "GET", config);
+ }
+
+ /**
+ * Execute a synchronous HTTP request using the specified method.
+ *
+ * <p>This method performs the HTTP transport operation and returns the response without
+ * interpreting the status code or parsing the response body.
+ *
+ * @param url the URL to request
+ * @param method the HTTP method
+ * @param config the HTTP configuration containing timeouts and headers
+ * @return the raw HTTP response
+ * @throws IOException if the HTTP request or response read fails
+ */
+ public static HttpResponse execute(String url, String method, HttpConfig config) throws IOException
+ {
+ if (logger.isDebugEnabled())
+ {
+ logger.debug("Executing HTTP {} request to URL: {}", method, url);
Review Comment:
Thanks, good point. I combined the request details into a single debug statement so they stay together during concurrent execution.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21195: Extract reusable HTTP utility from cloud metadata connector [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on PR #5219:
URL: https://github.com/apache/cassandra/pull/5219#issuecomment-5893931984
@bharathbikkavolu I will create one more version of apiCall method which will take `HttpConfig` so callers can inject there their own config with timeouts.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21195: Extract reusable HTTP utility from cloud metadata connector [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #5219:
URL: https://github.com/apache/cassandra/pull/5219#discussion_r4119549196
##########
src/java/org/apache/cassandra/locator/AbstractCloudMetadataServiceConnector.java:
##########
@@ -103,35 +100,16 @@ public String apiCall(String url,
Map<String, String> extraHeaders,
int expectedResponseCode) throws IOException
{
- HttpURLConnection conn = null;
Review Comment:
@bharathbikkavolu you can move HttpException inner class in this class into HttpUtil as well.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21195: Extract reusable HTTP utility from cloud metadata connector [cassandra]
Posted by "bharathbikkavolu (via GitHub)" <gi...@apache.org>.
bharathbikkavolu commented on code in PR #5219:
URL: https://github.com/apache/cassandra/pull/5219#discussion_r4129919163
##########
src/java/org/apache/cassandra/locator/AbstractCloudMetadataServiceConnector.java:
##########
@@ -103,35 +100,16 @@ public String apiCall(String url,
Map<String, String> extraHeaders,
int expectedResponseCode) throws IOException
{
- HttpURLConnection conn = null;
Review Comment:
Thanks @smiklosovic . HttpException has been moved into HttpService as part of the refactor.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21195: Extract reusable HTTP utility from cloud metadata connector [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #5219:
URL: https://github.com/apache/cassandra/pull/5219#discussion_r4121299098
##########
src/java/org/apache/cassandra/locator/AbstractCloudMetadataServiceConnector.java:
##########
Review Comment:
@bharathbikkavolu my idea is to create an abstract HttpService class and move all "api methods' there. There is no reason to leave these methods in this one. We should move everything related.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21195: Extract reusable HTTP utility from cloud metadata connector [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic commented on code in PR #5219:
URL: https://github.com/apache/cassandra/pull/5219#discussion_r4121458451
##########
src/java/org/apache/cassandra/locator/AbstractCloudMetadataServiceConnector.java:
##########
@@ -103,35 +100,16 @@ public String apiCall(String url,
Map<String, String> extraHeaders,
int expectedResponseCode) throws IOException
{
- HttpURLConnection conn = null;
Review Comment:
I created PR here against yours to extract even more stuff https://github.com/bharathbikkavolu/cassandra/pull/1
##########
src/java/org/apache/cassandra/locator/AbstractCloudMetadataServiceConnector.java:
##########
Review Comment:
https://github.com/bharathbikkavolu/cassandra/pull/1
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21195: Extract reusable HTTP utility from cloud metadata connector [cassandra]
Posted by "bharathbikkavolu (via GitHub)" <gi...@apache.org>.
bharathbikkavolu commented on code in PR #5219:
URL: https://github.com/apache/cassandra/pull/5219#discussion_r4129924405
##########
src/java/org/apache/cassandra/utils/HttpService.java:
##########
@@ -0,0 +1,284 @@
+/*
+ * 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.cassandra.utils;
+
+import java.io.DataInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.util.Map;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+/**
+ * Utility for making synchronous HTTP requests during Cassandra startup and initialization.
+ * This class provides reusable HTTP transport logic for components that need to retrieve
+ * information from external HTTP services during startup.
+ *
+ * <p>This utility is intended for startup-time operations where synchronous, blocking HTTP
+ * requests are appropriate. Examples include retrieving cloud metadata or configuration
+ * information required before Cassandra begins serving requests.
+ *
+ * <p><b>Design principles:</b>
+ * <ul>
+ * <li><b>Synchronous requests only</b> - Requests block until the operation completes or times out</li>
+ * <li><b>Raw HTTP responses</b> - Status-code validation and response parsing are the caller's responsibility</li>
+ * <li><b>Configurable timeouts</b> - Connection and read timeouts are supplied by the caller</li>
+ * <li><b>Header support</b> - Request headers can be supplied by the caller</li>
+ * <li><b>Diagnostic logging</b> - Request metadata is logged without logging header values or response contents</li>
+ * </ul>
+ *
+ * <p>This utility extracts common HTTP transport logic from components such as
+ * {@link org.apache.cassandra.locator.AbstractCloudMetadataServiceConnector} while preserving
+ * caller-specific response handling.
+ */
+public class HttpUtil
+{
+ private static final Logger logger = LoggerFactory.getLogger(HttpUtil.class);
+
+ /**
+ * Configuration for an HTTP request.
+ */
+ public static class HttpConfig
+ {
+ private final int connectTimeoutMs;
+ private final int readTimeoutMs;
+ private final Map<String, String> headers;
+
+ public HttpConfig(int connectTimeoutMs, int readTimeoutMs, Map<String, String> headers)
+ {
+ this.connectTimeoutMs = connectTimeoutMs;
+ this.readTimeoutMs = readTimeoutMs;
+ this.headers = headers != null ? headers : Map.of();
+ }
+
+ public int getConnectTimeoutMs()
+ {
+ return connectTimeoutMs;
+ }
+
+ public int getReadTimeoutMs()
+ {
+ return readTimeoutMs;
+ }
+
+ public Map<String, String> getHeaders()
+ {
+ return headers;
+ }
+ }
+
+ /**
+ * Raw HTTP response containing the status code, status message, and response body.
+ * Interpretation of the response is the caller's responsibility.
+ */
+ public static class HttpResponse
+ {
+ private final int statusCode;
+ private final String statusMessage;
+ private final String body;
+
+ public HttpResponse(int statusCode, String statusMessage, String body)
+ {
+ this.statusCode = statusCode;
+ this.statusMessage = statusMessage;
+ this.body = body;
+ }
+
+ public int getStatusCode()
+ {
+ return statusCode;
+ }
+
+ public String getStatusMessage()
+ {
+ return statusMessage;
+ }
+
+ public String getBody()
+ {
+ return body;
+ }
+ }
+
+ /**
+ * Execute a synchronous HTTP GET request.
+ *
+ * @param url the URL to request
+ * @param config the HTTP configuration containing timeouts and headers
+ * @return the raw HTTP response
+ * @throws IOException if the HTTP request or response read fails
+ */
+ public static HttpResponse executeGet(String url, HttpConfig config) throws IOException
+ {
+ logger.trace("Executing GET request via executeGet convenience method");
+ return execute(url, "GET", config);
+ }
+
+ /**
+ * Execute a synchronous HTTP request using the specified method.
+ *
+ * <p>This method performs the HTTP transport operation and returns the response without
+ * interpreting the status code or parsing the response body.
+ *
+ * @param url the URL to request
+ * @param method the HTTP method
+ * @param config the HTTP configuration containing timeouts and headers
+ * @return the raw HTTP response
+ * @throws IOException if the HTTP request or response read fails
+ */
+ public static HttpResponse execute(String url, String method, HttpConfig config) throws IOException
+ {
+ if (logger.isDebugEnabled())
+ {
+ logger.debug("Executing HTTP {} request to URL: {}", method, url);
+ logger.debug("Connection timeout: {}ms, Read timeout: {}ms",
+ config.getConnectTimeoutMs(), config.getReadTimeoutMs());
+ }
+
+ if (logger.isTraceEnabled() && !config.getHeaders().isEmpty())
+ {
+ logger.trace("Request header names: {}", config.getHeaders().keySet());
+ }
+
+ HttpURLConnection conn = null;
+ long startTime = System.currentTimeMillis();
+
+ try
+ {
+ conn = (HttpURLConnection) new URL(url).openConnection();
+ conn.setRequestMethod(method);
+ conn.setConnectTimeout(config.getConnectTimeoutMs());
+ conn.setReadTimeout(config.getReadTimeoutMs());
+
+ for (Map.Entry<String, String> header : config.getHeaders().entrySet())
+ {
+ conn.setRequestProperty(header.getKey(), header.getValue());
+
+ if (logger.isTraceEnabled())
+ {
+ logger.trace("Setting request header: {}", header.getKey());
+ }
+ }
+
+ logger.trace("Opening connection to {}", url);
+
+ int statusCode = conn.getResponseCode();
+ String statusMessage = conn.getResponseMessage();
+ long responseTime = System.currentTimeMillis() - startTime;
+
+ if (logger.isDebugEnabled())
+ {
+ logger.debug("HTTP {} request to {} completed in {}ms with status: {} {}",
+ method, url, responseTime, statusCode, statusMessage);
+ }
+
+ String responseBody = readResponseBody(conn, statusCode);
+
+ if (logger.isTraceEnabled())
+ {
+ int bodyLength = responseBody != null ? responseBody.length() : 0;
+ logger.trace("Response body length: {} bytes", bodyLength);
+ }
+
+ return new HttpResponse(statusCode, statusMessage, responseBody);
+ }
+ catch (IOException e)
+ {
+ long failureTime = System.currentTimeMillis() - startTime;
+ logger.error("HTTP {} request to {} failed after {}ms: {}",
+ method, url, failureTime, e.getMessage(), e);
+ throw e;
+ }
+ finally
+ {
+ if (conn != null)
+ {
+ logger.trace("Disconnecting HTTP connection to {}", url);
+ conn.disconnect();
+ }
+ }
+ }
+
+ /**
+ * Read the response body using the same content-length based behavior as
+ * AbstractCloudMetadataServiceConnector.
+ *
+ * @param conn the HTTP connection
Review Comment:
Thanks. I added the missing statusCode parameter documentation.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21195: Extract reusable HTTP utility from cloud metadata connector [cassandra]
Posted by "bharathbikkavolu (via GitHub)" <gi...@apache.org>.
bharathbikkavolu commented on code in PR #5219:
URL: https://github.com/apache/cassandra/pull/5219#discussion_r4129919163
##########
src/java/org/apache/cassandra/locator/AbstractCloudMetadataServiceConnector.java:
##########
@@ -103,35 +100,16 @@ public String apiCall(String url,
Map<String, String> extraHeaders,
int expectedResponseCode) throws IOException
{
- HttpURLConnection conn = null;
Review Comment:
Thanks Stefan. HttpException has been moved into HttpService as part of the refactor.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21195: Extract reusable HTTP utility from cloud metadata connector [cassandra]
Posted by "gandherva (via GitHub)" <gi...@apache.org>.
gandherva commented on code in PR #5219:
URL: https://github.com/apache/cassandra/pull/5219#discussion_r4129052949
##########
src/java/org/apache/cassandra/utils/HttpUtil.java:
##########
@@ -0,0 +1,284 @@
+/*
+ * 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.cassandra.utils;
+
+import java.io.DataInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.util.Map;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+/**
+ * Utility for making synchronous HTTP requests during Cassandra startup and initialization.
+ * This class provides reusable HTTP transport logic for components that need to retrieve
+ * information from external HTTP services during startup.
+ *
+ * <p>This utility is intended for startup-time operations where synchronous, blocking HTTP
+ * requests are appropriate. Examples include retrieving cloud metadata or configuration
+ * information required before Cassandra begins serving requests.
+ *
+ * <p><b>Design principles:</b>
+ * <ul>
+ * <li><b>Synchronous requests only</b> - Requests block until the operation completes or times out</li>
+ * <li><b>Raw HTTP responses</b> - Status-code validation and response parsing are the caller's responsibility</li>
+ * <li><b>Configurable timeouts</b> - Connection and read timeouts are supplied by the caller</li>
+ * <li><b>Header support</b> - Request headers can be supplied by the caller</li>
+ * <li><b>Diagnostic logging</b> - Request metadata is logged without logging header values or response contents</li>
+ * </ul>
+ *
+ * <p>This utility extracts common HTTP transport logic from components such as
+ * {@link org.apache.cassandra.locator.AbstractCloudMetadataServiceConnector} while preserving
+ * caller-specific response handling.
+ */
+public class HttpUtil
+{
+ private static final Logger logger = LoggerFactory.getLogger(HttpUtil.class);
+
+ /**
+ * Configuration for an HTTP request.
+ */
+ public static class HttpConfig
+ {
+ private final int connectTimeoutMs;
+ private final int readTimeoutMs;
+ private final Map<String, String> headers;
+
+ public HttpConfig(int connectTimeoutMs, int readTimeoutMs, Map<String, String> headers)
+ {
+ this.connectTimeoutMs = connectTimeoutMs;
+ this.readTimeoutMs = readTimeoutMs;
+ this.headers = headers != null ? headers : Map.of();
+ }
+
+ public int getConnectTimeoutMs()
+ {
+ return connectTimeoutMs;
+ }
+
+ public int getReadTimeoutMs()
+ {
+ return readTimeoutMs;
+ }
+
+ public Map<String, String> getHeaders()
+ {
+ return headers;
+ }
+ }
+
+ /**
+ * Raw HTTP response containing the status code, status message, and response body.
+ * Interpretation of the response is the caller's responsibility.
+ */
+ public static class HttpResponse
+ {
+ private final int statusCode;
+ private final String statusMessage;
+ private final String body;
+
+ public HttpResponse(int statusCode, String statusMessage, String body)
+ {
+ this.statusCode = statusCode;
+ this.statusMessage = statusMessage;
+ this.body = body;
+ }
+
+ public int getStatusCode()
+ {
+ return statusCode;
+ }
+
+ public String getStatusMessage()
+ {
+ return statusMessage;
+ }
+
+ public String getBody()
+ {
+ return body;
+ }
+ }
+
+ /**
+ * Execute a synchronous HTTP GET request.
+ *
+ * @param url the URL to request
+ * @param config the HTTP configuration containing timeouts and headers
+ * @return the raw HTTP response
+ * @throws IOException if the HTTP request or response read fails
+ */
+ public static HttpResponse executeGet(String url, HttpConfig config) throws IOException
+ {
+ logger.trace("Executing GET request via executeGet convenience method");
+ return execute(url, "GET", config);
+ }
+
+ /**
+ * Execute a synchronous HTTP request using the specified method.
+ *
+ * <p>This method performs the HTTP transport operation and returns the response without
+ * interpreting the status code or parsing the response body.
+ *
+ * @param url the URL to request
+ * @param method the HTTP method
+ * @param config the HTTP configuration containing timeouts and headers
+ * @return the raw HTTP response
+ * @throws IOException if the HTTP request or response read fails
+ */
+ public static HttpResponse execute(String url, String method, HttpConfig config) throws IOException
+ {
+ if (logger.isDebugEnabled())
+ {
+ logger.debug("Executing HTTP {} request to URL: {}", method, url);
Review Comment:
It would be good to combine these two log debug statements to a single one, so that they don't get separated due to multiple concurrent thread execution. Also good to have one log statement for one particular logical event(some exceptions apply)
##########
src/java/org/apache/cassandra/utils/HttpUtil.java:
##########
@@ -0,0 +1,284 @@
+/*
+ * 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.cassandra.utils;
+
+import java.io.DataInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.util.Map;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+/**
+ * Utility for making synchronous HTTP requests during Cassandra startup and initialization.
+ * This class provides reusable HTTP transport logic for components that need to retrieve
+ * information from external HTTP services during startup.
+ *
+ * <p>This utility is intended for startup-time operations where synchronous, blocking HTTP
+ * requests are appropriate. Examples include retrieving cloud metadata or configuration
+ * information required before Cassandra begins serving requests.
+ *
+ * <p><b>Design principles:</b>
+ * <ul>
+ * <li><b>Synchronous requests only</b> - Requests block until the operation completes or times out</li>
+ * <li><b>Raw HTTP responses</b> - Status-code validation and response parsing are the caller's responsibility</li>
+ * <li><b>Configurable timeouts</b> - Connection and read timeouts are supplied by the caller</li>
+ * <li><b>Header support</b> - Request headers can be supplied by the caller</li>
+ * <li><b>Diagnostic logging</b> - Request metadata is logged without logging header values or response contents</li>
+ * </ul>
+ *
+ * <p>This utility extracts common HTTP transport logic from components such as
+ * {@link org.apache.cassandra.locator.AbstractCloudMetadataServiceConnector} while preserving
+ * caller-specific response handling.
+ */
+public class HttpUtil
+{
+ private static final Logger logger = LoggerFactory.getLogger(HttpUtil.class);
+
+ /**
+ * Configuration for an HTTP request.
+ */
+ public static class HttpConfig
+ {
+ private final int connectTimeoutMs;
+ private final int readTimeoutMs;
+ private final Map<String, String> headers;
+
+ public HttpConfig(int connectTimeoutMs, int readTimeoutMs, Map<String, String> headers)
+ {
+ this.connectTimeoutMs = connectTimeoutMs;
+ this.readTimeoutMs = readTimeoutMs;
+ this.headers = headers != null ? headers : Map.of();
+ }
+
+ public int getConnectTimeoutMs()
+ {
+ return connectTimeoutMs;
+ }
+
+ public int getReadTimeoutMs()
+ {
+ return readTimeoutMs;
+ }
+
+ public Map<String, String> getHeaders()
+ {
+ return headers;
+ }
+ }
+
+ /**
+ * Raw HTTP response containing the status code, status message, and response body.
+ * Interpretation of the response is the caller's responsibility.
+ */
+ public static class HttpResponse
+ {
+ private final int statusCode;
+ private final String statusMessage;
+ private final String body;
+
+ public HttpResponse(int statusCode, String statusMessage, String body)
+ {
+ this.statusCode = statusCode;
+ this.statusMessage = statusMessage;
+ this.body = body;
+ }
+
+ public int getStatusCode()
+ {
+ return statusCode;
+ }
+
+ public String getStatusMessage()
+ {
+ return statusMessage;
+ }
+
+ public String getBody()
+ {
+ return body;
+ }
+ }
+
+ /**
+ * Execute a synchronous HTTP GET request.
+ *
+ * @param url the URL to request
+ * @param config the HTTP configuration containing timeouts and headers
+ * @return the raw HTTP response
+ * @throws IOException if the HTTP request or response read fails
+ */
+ public static HttpResponse executeGet(String url, HttpConfig config) throws IOException
+ {
+ logger.trace("Executing GET request via executeGet convenience method");
+ return execute(url, "GET", config);
+ }
+
+ /**
+ * Execute a synchronous HTTP request using the specified method.
+ *
+ * <p>This method performs the HTTP transport operation and returns the response without
+ * interpreting the status code or parsing the response body.
+ *
+ * @param url the URL to request
+ * @param method the HTTP method
+ * @param config the HTTP configuration containing timeouts and headers
+ * @return the raw HTTP response
+ * @throws IOException if the HTTP request or response read fails
+ */
+ public static HttpResponse execute(String url, String method, HttpConfig config) throws IOException
+ {
+ if (logger.isDebugEnabled())
+ {
+ logger.debug("Executing HTTP {} request to URL: {}", method, url);
+ logger.debug("Connection timeout: {}ms, Read timeout: {}ms",
+ config.getConnectTimeoutMs(), config.getReadTimeoutMs());
+ }
+
+ if (logger.isTraceEnabled() && !config.getHeaders().isEmpty())
+ {
+ logger.trace("Request header names: {}", config.getHeaders().keySet());
+ }
+
+ HttpURLConnection conn = null;
+ long startTime = System.currentTimeMillis();
+
+ try
+ {
+ conn = (HttpURLConnection) new URL(url).openConnection();
+ conn.setRequestMethod(method);
+ conn.setConnectTimeout(config.getConnectTimeoutMs());
+ conn.setReadTimeout(config.getReadTimeoutMs());
+
+ for (Map.Entry<String, String> header : config.getHeaders().entrySet())
+ {
+ conn.setRequestProperty(header.getKey(), header.getValue());
+
+ if (logger.isTraceEnabled())
+ {
+ logger.trace("Setting request header: {}", header.getKey());
+ }
+ }
+
+ logger.trace("Opening connection to {}", url);
+
+ int statusCode = conn.getResponseCode();
+ String statusMessage = conn.getResponseMessage();
+ long responseTime = System.currentTimeMillis() - startTime;
+
+ if (logger.isDebugEnabled())
+ {
+ logger.debug("HTTP {} request to {} completed in {}ms with status: {} {}",
+ method, url, responseTime, statusCode, statusMessage);
+ }
+
+ String responseBody = readResponseBody(conn, statusCode);
+
+ if (logger.isTraceEnabled())
+ {
+ int bodyLength = responseBody != null ? responseBody.length() : 0;
+ logger.trace("Response body length: {} bytes", bodyLength);
+ }
+
+ return new HttpResponse(statusCode, statusMessage, responseBody);
+ }
+ catch (IOException e)
+ {
+ long failureTime = System.currentTimeMillis() - startTime;
+ logger.error("HTTP {} request to {} failed after {}ms: {}",
+ method, url, failureTime, e.getMessage(), e);
+ throw e;
+ }
+ finally
+ {
+ if (conn != null)
+ {
+ logger.trace("Disconnecting HTTP connection to {}", url);
+ conn.disconnect();
+ }
+ }
+ }
+
+ /**
+ * Read the response body using the same content-length based behavior as
+ * AbstractCloudMetadataServiceConnector.
+ *
+ * @param conn the HTTP connection
Review Comment:
statusCode is missing on documentation
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSANDRA-21195: Extract reusable HTTP utility from cloud metadata connector [cassandra]
Posted by "smiklosovic (via GitHub)" <gi...@apache.org>.
smiklosovic closed pull request #5219: CASSANDRA-21195: Extract reusable HTTP utility from cloud metadata connector
URL: https://github.com/apache/cassandra/pull/5219
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org