xiangfu0 commented on code in PR #17167:
URL: https://github.com/apache/pinot/pull/17167#discussion_r3039174565
##########
pinot-connectors/pinot-flink-connector/src/main/java/org/apache/pinot/connector/flink/http/PinotConnectionUtils.java:
##########
@@ -38,18 +38,23 @@ public final class PinotConnectionUtils {
private PinotConnectionUtils() {
}
- public static Schema getSchema(ControllerRequestClient client, String
tableName) {
+ public static Schema getSchema(PinotAdminClient client, String tableName) {
try {
- return client.getSchema(tableName);
+ return client.getSchemaClient().getSchema(tableName);
} catch (Exception e) {
throw new RuntimeException(String.format("Failed to get table schema %s
from Pinot controller", tableName), e);
}
}
- public static TableConfig getTableConfig(ControllerRequestClient client,
String tableName, String tableType) {
+ public static TableConfig getTableConfig(PinotAdminClient client, String
tableName, String tableType) {
TableConfig tableConfig;
try {
- tableConfig = client.getTableConfig(tableName,
TableType.valueOf(tableType));
+ String configJson = client.getTableClient().getTableConfig(tableName,
tableType);
Review Comment:
Done - PinotTableAdminClient has getTableConfigObject() returning typed
TableConfig.
##########
pinot-connectors/pinot-flink-connector/src/main/java/org/apache/pinot/connector/flink/FlinkQuickStart.java:
##########
@@ -79,12 +77,11 @@ public static void main(String[] args)
execEnv.setParallelism(2);
DataStream<Row> srcDs =
execEnv.fromCollection(data).returns(TEST_TYPE_INFO).keyBy(r -> r.getField(0));
- HttpClient httpClient = HttpClient.getInstance();
- ControllerRequestClient client = new ControllerRequestClient(
- ControllerRequestURLBuilder.baseUrl(DEFAULT_CONTROLLER_URL),
httpClient);
- Schema schema = PinotConnectionUtils.getSchema(client, "starbucksStores");
- TableConfig tableConfig = PinotConnectionUtils.getTableConfig(client,
"starbucksStores", "OFFLINE");
- srcDs.addSink(new PinotSinkFunction<>(new
FlinkRowGenericRowConverter(TEST_TYPE_INFO), tableConfig, schema));
- execEnv.execute();
+ try (PinotAdminClient client = new PinotAdminClient("localhost:9000")) {
+ Schema schema = client.getSchemaClient().getSchema("starbucksStores");
+ TableConfig tableConfig = PinotConnectionUtils.getTableConfig(client,
"starbucksStores", "OFFLINE");
Review Comment:
Done - PinotConnectionUtils removed; FlinkQuickStart uses PinotAdminClient
directly.
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableViews.java:
##########
@@ -351,10 +352,19 @@ private TableView getTableIdealState(String
tableNameOptType, @Nullable TableTyp
private TableView getTableExternalView(String tableNameOptType, @Nullable
TableType tableType) {
TableView tableView = new TableView();
if (tableType == null || tableType == TableType.OFFLINE) {
- tableView._offline = getExternalView(tableNameOptType,
TableType.OFFLINE);
+ Map<String, Map<String, String>> external =
getExternalView(tableNameOptType, TableType.OFFLINE);
+ if (external == null || external.isEmpty()) {
+ // Fallback to ideal state if external view is not yet materialized.
+ external = getIdealState(tableNameOptType, TableType.OFFLINE);
Review Comment:
Acknowledged - TableViews controller code is unchanged in this PR (only a
cosmetic blank line). No idealstate fallback was added.
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableViews.java:
##########
@@ -351,10 +352,19 @@ private TableView getTableIdealState(String
tableNameOptType, @Nullable TableTyp
private TableView getTableExternalView(String tableNameOptType, @Nullable
TableType tableType) {
TableView tableView = new TableView();
if (tableType == null || tableType == TableType.OFFLINE) {
- tableView._offline = getExternalView(tableNameOptType,
TableType.OFFLINE);
+ Map<String, Map<String, String>> external =
getExternalView(tableNameOptType, TableType.OFFLINE);
+ if (external == null || external.isEmpty()) {
+ // Fallback to ideal state if external view is not yet materialized.
+ external = getIdealState(tableNameOptType, TableType.OFFLINE);
+ }
+ tableView._offline = external;
}
if (tableType == null || tableType == TableType.REALTIME) {
- tableView._realtime = getExternalView(tableNameOptType,
TableType.REALTIME);
+ Map<String, Map<String, String>> external =
getExternalView(tableNameOptType, TableType.REALTIME);
+ if (external == null || external.isEmpty()) {
+ external = getIdealState(tableNameOptType, TableType.REALTIME);
Review Comment:
Acknowledged - no idealstate fallback in the current code.
##########
pinot-connectors/pinot-flink-connector/src/main/java/org/apache/pinot/connector/flink/http/PinotConnectionUtils.java:
##########
@@ -38,18 +38,23 @@ public final class PinotConnectionUtils {
private PinotConnectionUtils() {
}
- public static Schema getSchema(ControllerRequestClient client, String
tableName) {
+ public static Schema getSchema(PinotAdminClient client, String tableName) {
Review Comment:
Done - PinotConnectionUtils removed.
##########
pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotIngestionRestletResourceStatelessTest.java:
##########
@@ -100,13 +102,14 @@ public void testIngestEndpoint()
Map<String, String> batchConfigMap = new HashMap<>();
batchConfigMap.put(BatchConfigProperties.INPUT_FORMAT, "csv");
batchConfigMap.put(String.format("%s.delimiter",
BatchConfigProperties.RECORD_READER_PROP_PREFIX), "|");
-
sendHttpPost(_controllerRequestURLBuilder.forIngestFromFile(TABLE_NAME_WITH_TYPE,
batchConfigMap));
+
sendHttpPost(adminClient.getTableClient().buildIngestFromFileUrl(TABLE_NAME_WITH_TYPE,
batchConfigMap));
Review Comment:
Done - PinotFileIngestClient created with
ingestFromFile/ingestFromUri/postFile/postString methods.
##########
pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotFileUploadTest.java:
##########
@@ -58,8 +58,9 @@ public void setUp()
@Test
public void testUploadBogusData()
throws Exception {
+ String uploadUrl = TEST_INSTANCE.getAdminClient().getSegmentUploadUrl();
try (CloseableHttpClient httpClient = HttpClients.createDefault()) {
- HttpPost httpPost = new
HttpPost(TEST_INSTANCE.getControllerRequestURLBuilder().forDataFileUpload());
+ HttpPost httpPost = new HttpPost(uploadUrl);
Review Comment:
Done - PinotFileIngestClient already created.
##########
pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotInstanceRestletResourceTest.java:
##########
@@ -274,23 +271,20 @@ public void instanceRetagServerDeficiencyTest()
}
private Map<String, List<String>> getCurrentInstanceTagsMap()
- throws IOException {
- String listInstancesUrl = _urlBuilder.forInstanceList();
- JsonNode response =
JsonUtils.stringToJsonNode(sendGetRequest(listInstancesUrl));
- JsonNode instances = response.get("instances");
+ throws Exception {
+ PinotAdminClient adminClient = DEFAULT_INSTANCE.getOrCreateAdminClient();
+ List<String> instances = adminClient.getInstanceClient().listInstances();
Map<String, List<String>> map = new HashMap<>(instances.size());
- for (int i = 0; i < instances.size(); i++) {
- String instance = instances.get(i).asText();
- map.put(instance, getInstanceTags(instance));
+ for (String instance : instances) {
+ map.put(instance, getInstanceTags(adminClient, instance));
}
return map;
}
- private List<String> getInstanceTags(String instance)
- throws IOException {
- String getInstancesUrl = _urlBuilder.forInstance(instance);
+ private List<String> getInstanceTags(PinotAdminClient adminClient, String
instance)
+ throws Exception {
List<String> tags = new ArrayList<>();
- for (JsonNode tag :
JsonUtils.stringToJsonNode(sendGetRequest(getInstancesUrl)).get("tags")) {
+ for (JsonNode tag :
JsonUtils.stringToJsonNode(adminClient.getInstanceClient().getInstance(instance)).get("tags"))
{
Review Comment:
Done - getInstanceTags(String) already added to PinotInstanceAdminClient.
--
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]
