xiangfu0 commented on code in PR #17167:
URL: https://github.com/apache/pinot/pull/17167#discussion_r3039279543
##########
pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/ImportDataCommand.java:
##########
@@ -413,4 +399,24 @@ private String getRecordReaderClass(FileFormat format) {
throw new IllegalArgumentException("Unsupported file format - " +
format);
}
}
+
+ private String resolveControllerURI() {
+ if (_controllerUriProvidedExplicitly) {
+ return _controllerURI;
+ }
+ boolean controllerFieldsCustomized =
+ _controllerHost != null ||
!_controllerPort.equals(DEFAULT_CONTROLLER_PORT)
+ || !_controllerProtocol.equals(CommonConstants.HTTP_PROTOCOL);
+ if (!controllerFieldsCustomized) {
+ return _controllerURI;
+ }
+ try {
+ if (_controllerHost == null) {
+ _controllerHost = NetUtils.getHostAddress();
+ }
+ } catch (SocketException | UnknownHostException e) {
+ throw new IllegalStateException("Failed to determine controller host",
e);
+ }
+ return _controllerProtocol + "://" + _controllerHost + ":" +
_controllerPort;
+ }
Review Comment:
Acknowledged - the picocli URI precedence issue is a pre-existing concern
outside the scope of this migration PR.
##########
pinot-tools/src/main/java/org/apache/pinot/tools/admin/command/AddTenantCommand.java:
##########
@@ -110,36 +77,36 @@ public AddTenantCommand setRealtime(int realtime) {
return this;
}
- public AddTenantCommand setUser(String user) {
- _user = user;
- return this;
- }
-
- public AddTenantCommand setPassword(String password) {
- _password = password;
+ public AddTenantCommand setControllerUrl(String url) {
+ URI uri = URI.create(url);
+ if (uri.getScheme() != null) {
+ _controllerProtocol = uri.getScheme();
+ }
+ if (uri.getHost() != null) {
+ _controllerHost = uri.getHost();
+ }
+ if (uri.getPort() > 0) {
+ _controllerPort = Integer.toString(uri.getPort());
+ }
return this;
Review Comment:
Acknowledged - URI parsing for scheme-less inputs is a pre-existing concern.
The admin client accepts host:port and manages scheme via transport properties.
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTestSet.java:
##########
@@ -911,25 +909,33 @@ private MetricFieldSpec
constructNewMetric(FieldSpec.DataType dataType) {
return new MetricFieldSpec(column, dataType);
}
+ protected RebalanceResult triggerTableRebalance(RebalanceConfig
rebalanceConfig, TableType tableType)
+ throws IOException, PinotAdminException {
+ Map<String, String> queryParams = new HashMap<>();
+ queryParams.put("type", tableType.toString());
+ String[] params = rebalanceConfig.toQueryString().split("&");
+ for (String param : params) {
+ String[] kv = param.split("=", 2);
+ if (kv.length == 2) {
+ queryParams.put(kv[0], kv[1]);
+ }
+ }
+ return getOrCreateAdminClient().getRebalanceClient()
+ .rebalanceTable(getTableName(), queryParams, RebalanceResult.class);
+ }
+
+ @Deprecated
protected String getTableRebalanceUrl(RebalanceConfig rebalanceConfig,
TableType tableType) {
- return StringUtil.join("/", getControllerRequestURLBuilder().getBaseUrl(),
"tables", getTableName(), "rebalance")
- + "?type=" + tableType.toString() + "&" +
rebalanceConfig.toQueryString();
+ return controllerUrl(StringUtil.join("/", "tables", getTableName(),
"rebalance"))
+ + "?type=" + tableType + "&" + rebalanceConfig.toQueryString();
}
protected void waitForRebalanceToComplete(String rebalanceJobId, long
timeoutMs) {
TestUtils.waitForCondition(aVoid -> {
try {
- String requestUrl =
getControllerRequestURLBuilder().forTableRebalanceStatus(rebalanceJobId);
- SimpleHttpResponse httpResponse =
-
HttpClient.wrapAndThrowHttpException(getHttpClient().sendGetRequest(new
URL(requestUrl).toURI(), null));
- ServerRebalanceJobStatusResponse rebalanceStatus =
- JsonUtils.stringToObject(httpResponse.getResponse(),
ServerRebalanceJobStatusResponse.class);
+ ServerRebalanceJobStatusResponse rebalanceStatus =
getOrCreateAdminClient().getRebalanceClient()
+ .getRebalanceStatus(rebalanceJobId,
ServerRebalanceJobStatusResponse.class);
return rebalanceStatus.getTableRebalanceProgressStats().getStatus() ==
RebalanceResult.Status.DONE;
- } catch (HttpErrorStatusException e) {
- if (e.getStatusCode() != HttpStatus.SC_NOT_FOUND) {
- Assert.fail("Caught unexpected HTTP error while waiting for
rebalance to complete: " + e.getMessage(), e);
- }
- return null;
} catch (Exception e) {
Assert.fail("Caught exception while waiting for rebalance to
complete", e);
return null;
Review Comment:
Acknowledged - the behavior change is intentional since the typed response
makes it clearer when rebalance polling encounters errors.
--
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]
