wuchong commented on code in PR #20298:
URL: https://github.com/apache/flink/pull/20298#discussion_r931843918
##########
flink-connectors/flink-connector-hive/src/main/java/org/apache/flink/table/endpoint/hive/HiveServer2Endpoint.java:
##########
@@ -628,7 +628,9 @@ private void buildTThreadPoolServer() {
new TThreadPoolServer.Args(
new TServerSocket(
new ServerSocket(
- port, -1,
InetAddress.getByName(host))))
+ port,
+ -1,
+
InetAddress.getByName(hostAddress))))
Review Comment:
`hostAddress` has been resolved, we don't need to resolve again.
##########
flink-connectors/flink-connector-hive/src/main/java/org/apache/flink/table/endpoint/hive/HiveServer2EndpointFactory.java:
##########
@@ -77,21 +80,33 @@ public String factoryIdentifier() {
@Override
public Set<ConfigOption<?>> requiredOptions() {
+ return Collections.emptySet();
+ }
+
+ @Override
+ public Set<ConfigOption<?>> optionalOptions() {
return new HashSet<>(
Arrays.asList(
+ THRIFT_HOST,
THRIFT_PORT,
THRIFT_MAX_MESSAGE_SIZE,
THRIFT_LOGIN_TIMEOUT,
THRIFT_WORKER_THREADS_MIN,
THRIFT_WORKER_THREADS_MAX,
THRIFT_WORKER_KEEPALIVE_TIME,
CATALOG_NAME,
+ CATALOG_HIVE_CONF_DIR,
+ CATALOG_DEFAULT_DATABASE,
MODULE_NAME));
}
- @Override
- public Set<ConfigOption<?>> optionalOptions() {
- return new HashSet<>(Arrays.asList(CATALOG_HIVE_CONF_DIR,
CATALOG_DEFAULT_DATABASE));
+ private static String getHostAddress(String hostName) {
+ try {
+ return InetAddress.getByName(hostName).getHostAddress();
+ } catch (UnknownHostException e) {
+ throw new ValidationException(
+ String.format("Can not get the address for the host
'%s'.", hostName));
Review Comment:
throw the root exception
##########
flink-table/flink-sql-gateway/src/main/java/org/apache/flink/table/gateway/service/operation/OperationManager.java:
##########
@@ -315,7 +315,7 @@ public ResultSet fetchResults(FetchOrientation orientation,
int maxRows) {
public ResolvedSchema getResultSchema() {
OperationStatus current = status.get();
- if (current != OperationStatus.FINISHED || !hasResults) {
Review Comment:
What's the usage of `hasResults`? It seems never used. Can we remove the
variable?
##########
flink-connectors/flink-connector-hive/src/test/java/org/apache/flink/table/endpoint/hive/HiveServer2EndpointFactoryTest.java:
##########
@@ -79,7 +81,7 @@ public void testCreateHiveServer2Endpoint() {
Collections.singletonList(
new HiveServer2Endpoint(
service,
- "localhost",
+ "127.0.0.1",
Review Comment:
The ip address of "localhost" might be different across machines. So this
test is not deterministic.
--
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]
