Copilot commented on code in PR #369:
URL:
https://github.com/apache/doris-spark-connector/pull/369#discussion_r3733854867
##########
spark-doris-connector/spark-doris-connector-base/src/main/java/org/apache/doris/spark/client/read/DorisFlightSqlReader.java:
##########
@@ -148,17 +153,58 @@ public void close() {
log.warn("close adbc connection error", e);
}
}
+ try {
+ frontendClient.close();
+ } catch (IOException e) {
+ log.warn("close frontend client error", e);
+ }
}
private AdbcConnection initializeConnection(Frontend frontend, DorisConfig
config) throws OptionRequiredException, AdbcException {
BufferAllocator allocator = new RootAllocator();
FlightSqlDriver driver = new FlightSqlDriver(allocator);
+ DorisTlsOptions tlsOptions = config.getTlsOptions();
+ InputStream rootCertificates = null;
+ if (tlsOptions.isEnabledFor(DorisTlsOptions.Protocol.ARROW_FLIGHT)
+ && !tlsOptions.getCaCertificatePath().isEmpty()) {
+ rootCertificates =
DorisTlsContextFactory.openCaCertificate(tlsOptions);
+ }
+ try (InputStream certificates = rootCertificates) {
+ Map<String, Object> params = createConnectionParameters(
+ frontend.getHost(),
+ frontend.getFlightSqlPort(),
+ config,
+ certificates);
+ AdbcDatabase database = driver.open(params);
+ return database.connect();
+ } catch (IOException e) {
+ throw new DorisRuntimeException("Unable to close the Doris TLS CA
certificate", e);
+ }
Review Comment:
The `IOException` catch in `initializeConnection` always wraps failures with
the message "Unable to close the Doris TLS CA certificate", but the error is
specifically about closing the CA certificate stream (and could also surface as
an I/O error during resource cleanup). Tweaking the message to reference
closing the stream makes failures easier to interpret when diagnosing Flight
SQL TLS issues.
##########
spark-doris-connector/spark-doris-connector-base/src/main/java/org/apache/doris/spark/util/HttpUtil.java:
##########
@@ -17,40 +17,54 @@
package org.apache.doris.spark.util;
+import org.apache.doris.spark.config.DorisTlsOptions;
+import org.apache.doris.spark.exception.DorisRuntimeException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.io.IOException;
import java.net.HttpURLConnection;
import java.net.URL;
public class HttpUtil {
private static final Logger LOG = LoggerFactory.getLogger(HttpUtil.class);
public static boolean tryHttpConnection(String host) {
+ return tryHttpConnection(host, null);
+ }
Review Comment:
`tryHttpConnection(String host)` now propagates `DorisRuntimeException` from
the TLS-aware overload, which changes the long-standing contract of this
boolean “try” probe (it used to return `false` on failures). This can break any
existing callers that rely on a simple true/false health check and don’t expect
runtime exceptions.
Consider keeping the 1-arg overload backward-compatible by catching the
runtime failure and returning `false` (leaving the 2-arg overload as the
throwing variant if desired).
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
