You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/04/12 16:56:48 UTC

[GitHub] [pinot] saurabhd336 opened a new pull request, #8514: Indicate what fields are getting ignored when adding configs (WIP)

saurabhd336 opened a new pull request, #8514:
URL: https://github.com/apache/pinot/pull/8514

   Instructions:
   1. The PR has to be tagged with at least one of the following labels (*):
      1. `feature`
      2. `bugfix`
      3. `performance`
      4. `ui`
      5. `backward-incompat`
      6. `release-notes` (**)
   2. Remove these instructions before publishing the PR.
    
   (*) Other labels to consider:
   - `testing`
   - `dependencies`
   - `docker`
   - `kubernetes`
   - `observability`
   - `security`
   - `code-style`
   - `extension-point`
   - `refactor`
   - `cleanup`
   
   (**) Use `release-notes` label for scenarios like:
   - New configuration options
   - Deprecation of configurations
   - Signature changes to public methods/interfaces
   - New plugins added or old plugins removed
   


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] saurabhd336 commented on a diff in pull request #8514: Indicate what fields are getting ignored when adding configs

Posted by GitBox <gi...@apache.org>.
saurabhd336 commented on code in PR #8514:
URL: https://github.com/apache/pinot/pull/8514#discussion_r855764190


##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/JsonUtils.java:
##########
@@ -71,12 +88,53 @@ private JsonUtils() {
   public static final ObjectReader DEFAULT_READER = DEFAULT_MAPPER.reader();
   public static final ObjectWriter DEFAULT_WRITER = DEFAULT_MAPPER.writer();
   public static final ObjectWriter DEFAULT_PRETTY_WRITER = DEFAULT_MAPPER.writerWithDefaultPrettyPrinter();
+  private static final TypeReference<HashMap<String, Object>> GENERIC_JSON_TYPE =
+      new TypeReference<HashMap<String, Object>>() { };
 
   public static <T> T stringToObject(String jsonString, Class<T> valueType)
       throws IOException {
     return DEFAULT_READER.forType(valueType).readValue(jsonString);
   }
 
+  public static <T> JsonPojoWithUnparsableProps<T> stringToObjectAndUnparseableProps(String jsonString, Class<T> klass)

Review Comment:
   Ack on #1
   #2 The Pair class defined within the project expects 'T' to be extending Serializable, which would mean changing the base response classes. Other Pair implementations would need dependencies. Leaving it as it is for now.. Will revisit.



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] saurabhd336 commented on a diff in pull request #8514: Indicate what fields are getting ignored when adding configs

Posted by GitBox <gi...@apache.org>.
saurabhd336 commented on code in PR #8514:
URL: https://github.com/apache/pinot/pull/8514#discussion_r855764664


##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/JsonUtils.java:
##########
@@ -71,12 +88,53 @@ private JsonUtils() {
   public static final ObjectReader DEFAULT_READER = DEFAULT_MAPPER.reader();
   public static final ObjectWriter DEFAULT_WRITER = DEFAULT_MAPPER.writer();
   public static final ObjectWriter DEFAULT_PRETTY_WRITER = DEFAULT_MAPPER.writerWithDefaultPrettyPrinter();
+  private static final TypeReference<HashMap<String, Object>> GENERIC_JSON_TYPE =
+      new TypeReference<HashMap<String, Object>>() { };
 
   public static <T> T stringToObject(String jsonString, Class<T> valueType)
       throws IOException {
     return DEFAULT_READER.forType(valueType).readValue(jsonString);
   }
 
+  public static <T> JsonPojoWithUnparsableProps<T> stringToObjectAndUnparseableProps(String jsonString, Class<T> klass)
+      throws IOException {
+    T instance = DEFAULT_READER.forType(klass).readValue(jsonString);
+
+    String instanceJson = DEFAULT_MAPPER.writeValueAsString(instance);
+    Map<String, Object> inputJsonMap = flatten(DEFAULT_MAPPER.readValue(jsonString, GENERIC_JSON_TYPE));
+    Map<String, Object> instanceJsonMap = flatten(DEFAULT_MAPPER.readValue(instanceJson, GENERIC_JSON_TYPE));
+
+    MapDifference<String, Object> difference = Maps.difference(inputJsonMap, instanceJsonMap);
+    return new JsonPojoWithUnparsableProps<>(instance, difference.entriesOnlyOnLeft());
+  }
+
+  public static Map<String, Object> flatten(Map<String, Object> map) {
+    return map.entrySet().stream()
+        .flatMap(JsonUtils::flatten)
+        .collect(LinkedHashMap::new, (m, e) -> m.put("/" + e.getKey(), e.getValue()), LinkedHashMap::putAll);
+  }
+
+  private static Stream<Map.Entry<String, Object>> flatten(Map.Entry<String, Object> entry) {
+

Review Comment:
   Ack



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] saurabhd336 commented on a diff in pull request #8514: Indicate what fields are getting ignored when adding configs

Posted by GitBox <gi...@apache.org>.
saurabhd336 commented on code in PR #8514:
URL: https://github.com/apache/pinot/pull/8514#discussion_r859367467


##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/JsonUtils.java:
##########
@@ -71,12 +79,53 @@ private JsonUtils() {
   public static final ObjectReader DEFAULT_READER = DEFAULT_MAPPER.reader();
   public static final ObjectWriter DEFAULT_WRITER = DEFAULT_MAPPER.writer();
   public static final ObjectWriter DEFAULT_PRETTY_WRITER = DEFAULT_MAPPER.writerWithDefaultPrettyPrinter();
+  private static final TypeReference<HashMap<String, Object>> GENERIC_JSON_TYPE =
+      new TypeReference<HashMap<String, Object>>() { };
 
   public static <T> T stringToObject(String jsonString, Class<T> valueType)
       throws IOException {
     return DEFAULT_READER.forType(valueType).readValue(jsonString);
   }
 
+  public static <T> Pair<T, Map<String, Object>> stringToObjectAndUnparseableProps(String jsonString,

Review Comment:
   Ack



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #8514: Indicate what fields are getting ignored when adding configs

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #8514:
URL: https://github.com/apache/pinot/pull/8514#discussion_r856470862


##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/JsonUtils.java:
##########
@@ -71,12 +88,53 @@ private JsonUtils() {
   public static final ObjectReader DEFAULT_READER = DEFAULT_MAPPER.reader();
   public static final ObjectWriter DEFAULT_WRITER = DEFAULT_MAPPER.writer();
   public static final ObjectWriter DEFAULT_PRETTY_WRITER = DEFAULT_MAPPER.writerWithDefaultPrettyPrinter();
+  private static final TypeReference<HashMap<String, Object>> GENERIC_JSON_TYPE =
+      new TypeReference<HashMap<String, Object>>() { };
 
   public static <T> T stringToObject(String jsonString, Class<T> valueType)
       throws IOException {
     return DEFAULT_READER.forType(valueType).readValue(jsonString);
   }
 
+  public static <T> JsonPojoWithUnparsableProps<T> stringToObjectAndUnparseableProps(String jsonString, Class<T> klass)

Review Comment:
   Can we use `org.apache.commons.lang3.tuple.Pair`? I think we should already have the dependency



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] saurabhd336 commented on a diff in pull request #8514: Indicate what fields are getting ignored when adding configs

Posted by GitBox <gi...@apache.org>.
saurabhd336 commented on code in PR #8514:
URL: https://github.com/apache/pinot/pull/8514#discussion_r859368002


##########
pinot-spi/src/test/java/org/apache/pinot/spi/utils/JsonUtilsTest.java:
##########
@@ -273,6 +274,20 @@ public void testFlatten()
     }
   }
 
+  @Test
+  public void testUnparseableJson() throws Exception {

Review Comment:
   Ack



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] saurabhd336 commented on a diff in pull request #8514: Indicate what fields are getting ignored when adding configs

Posted by GitBox <gi...@apache.org>.
saurabhd336 commented on code in PR #8514:
URL: https://github.com/apache/pinot/pull/8514#discussion_r855760869


##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/ConfigSuccessResponse.java:
##########
@@ -0,0 +1,34 @@
+/**
+ * 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.pinot.controller.api.resources;
+
+import java.util.Map;
+
+public final class ConfigSuccessResponse extends SuccessResponse {
+  private final Map<String, Object> _unparseableProps;

Review Comment:
   Ack



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] npawar merged pull request #8514: Indicate what fields are getting ignored when adding configs

Posted by GitBox <gi...@apache.org>.
npawar merged PR #8514:
URL: https://github.com/apache/pinot/pull/8514


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #8514: Indicate what fields are getting ignored when adding configs

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #8514:
URL: https://github.com/apache/pinot/pull/8514#discussion_r855453394


##########
pinot-spi/src/test/java/org/apache/pinot/spi/utils/TestClass.java:
##########
@@ -0,0 +1,84 @@
+/**
+ * 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.pinot.spi.utils;
+
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+
+
+@JsonIgnoreProperties(ignoreUnknown = true)
+public class TestClass {
+  int _primitiveIntegerField;
+  String _stringField;
+  Long _longField;
+  Klass _classField;

Review Comment:
   (minor) I think we use `Clazz` in most of the places



##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/JsonUtils.java:
##########
@@ -71,12 +88,53 @@ private JsonUtils() {
   public static final ObjectReader DEFAULT_READER = DEFAULT_MAPPER.reader();
   public static final ObjectWriter DEFAULT_WRITER = DEFAULT_MAPPER.writer();
   public static final ObjectWriter DEFAULT_PRETTY_WRITER = DEFAULT_MAPPER.writerWithDefaultPrettyPrinter();
+  private static final TypeReference<HashMap<String, Object>> GENERIC_JSON_TYPE =
+      new TypeReference<HashMap<String, Object>>() { };
 
   public static <T> T stringToObject(String jsonString, Class<T> valueType)
       throws IOException {
     return DEFAULT_READER.forType(valueType).readValue(jsonString);
   }
 
+  public static <T> JsonPojoWithUnparsableProps<T> stringToObjectAndUnparseableProps(String jsonString, Class<T> klass)

Review Comment:
   (optional) I feel returning a `Pair<T, Map<String, Object>>` should be enough



##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/JsonUtils.java:
##########
@@ -71,12 +88,53 @@ private JsonUtils() {
   public static final ObjectReader DEFAULT_READER = DEFAULT_MAPPER.reader();
   public static final ObjectWriter DEFAULT_WRITER = DEFAULT_MAPPER.writer();
   public static final ObjectWriter DEFAULT_PRETTY_WRITER = DEFAULT_MAPPER.writerWithDefaultPrettyPrinter();
+  private static final TypeReference<HashMap<String, Object>> GENERIC_JSON_TYPE =
+      new TypeReference<HashMap<String, Object>>() { };
 
   public static <T> T stringToObject(String jsonString, Class<T> valueType)
       throws IOException {
     return DEFAULT_READER.forType(valueType).readValue(jsonString);
   }
 
+  public static <T> JsonPojoWithUnparsableProps<T> stringToObjectAndUnparseableProps(String jsonString, Class<T> klass)

Review Comment:
   Name the second parameter `valueType` for consistency



##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/JsonUtils.java:
##########
@@ -71,12 +88,53 @@ private JsonUtils() {
   public static final ObjectReader DEFAULT_READER = DEFAULT_MAPPER.reader();
   public static final ObjectWriter DEFAULT_WRITER = DEFAULT_MAPPER.writer();
   public static final ObjectWriter DEFAULT_PRETTY_WRITER = DEFAULT_MAPPER.writerWithDefaultPrettyPrinter();
+  private static final TypeReference<HashMap<String, Object>> GENERIC_JSON_TYPE =
+      new TypeReference<HashMap<String, Object>>() { };
 
   public static <T> T stringToObject(String jsonString, Class<T> valueType)
       throws IOException {
     return DEFAULT_READER.forType(valueType).readValue(jsonString);
   }
 
+  public static <T> JsonPojoWithUnparsableProps<T> stringToObjectAndUnparseableProps(String jsonString, Class<T> klass)
+      throws IOException {
+    T instance = DEFAULT_READER.forType(klass).readValue(jsonString);
+
+    String instanceJson = DEFAULT_MAPPER.writeValueAsString(instance);
+    Map<String, Object> inputJsonMap = flatten(DEFAULT_MAPPER.readValue(jsonString, GENERIC_JSON_TYPE));
+    Map<String, Object> instanceJsonMap = flatten(DEFAULT_MAPPER.readValue(instanceJson, GENERIC_JSON_TYPE));
+
+    MapDifference<String, Object> difference = Maps.difference(inputJsonMap, instanceJsonMap);
+    return new JsonPojoWithUnparsableProps<>(instance, difference.entriesOnlyOnLeft());
+  }
+
+  public static Map<String, Object> flatten(Map<String, Object> map) {
+    return map.entrySet().stream()
+        .flatMap(JsonUtils::flatten)
+        .collect(LinkedHashMap::new, (m, e) -> m.put("/" + e.getKey(), e.getValue()), LinkedHashMap::putAll);
+  }
+
+  private static Stream<Map.Entry<String, Object>> flatten(Map.Entry<String, Object> entry) {
+

Review Comment:
   (nit) remove empty line



##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/ConfigSuccessResponse.java:
##########
@@ -0,0 +1,34 @@
+/**
+ * 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.pinot.controller.api.resources;
+
+import java.util.Map;
+
+public final class ConfigSuccessResponse extends SuccessResponse {
+  private final Map<String, Object> _unparseableProps;

Review Comment:
   Suggest renaming the field, same for other places
   ```suggestion
     private final Map<String, Object> _ unrecognizedProperties;
   ```



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] codecov-commenter commented on pull request #8514: Indicate what fields are getting ignored when adding configs (WIP)

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #8514:
URL: https://github.com/apache/pinot/pull/8514#issuecomment-1097911025

   # [Codecov](https://codecov.io/gh/apache/pinot/pull/8514?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#8514](https://codecov.io/gh/apache/pinot/pull/8514?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1030b4b) into [master](https://codecov.io/gh/apache/pinot/commit/44da1a098fd47ffb533da6ca1386c64fb86e50a9?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (44da1a0) will **decrease** coverage by `44.59%`.
   > The diff coverage is `41.36%`.
   
   > :exclamation: Current head 1030b4b differs from pull request most recent head fc36bdf. Consider uploading reports for the commit fc36bdf to get more accurate results
   
   ```diff
   @@              Coverage Diff              @@
   ##             master    #8514       +/-   ##
   =============================================
   - Coverage     70.59%   26.00%   -44.60%     
   =============================================
     Files          1674     1663       -11     
     Lines         87618    87424      -194     
     Branches      13272    13243       -29     
   =============================================
   - Hits          61852    22732    -39120     
   - Misses        21477    62518    +41041     
   + Partials       4289     2174     -2115     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | integration1 | `?` | |
   | integration2 | `26.00% <41.36%> (+0.11%)` | :arrow_up: |
   | unittests1 | `?` | |
   | unittests2 | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/pinot/pull/8514?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...roller/api/resources/ConfigValidationResponse.java](https://codecov.io/gh/apache/pinot/pull/8514/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL0NvbmZpZ1ZhbGlkYXRpb25SZXNwb25zZS5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [...oller/api/resources/PinotTableRestletResource.java](https://codecov.io/gh/apache/pinot/pull/8514/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VGFibGVSZXN0bGV0UmVzb3VyY2UuamF2YQ==) | `26.80% <0.00%> (-23.37%)` | :arrow_down: |
   | [...ler/api/resources/TableConfigsRestletResource.java](https://codecov.io/gh/apache/pinot/pull/8514/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1RhYmxlQ29uZmlnc1Jlc3RsZXRSZXNvdXJjZS5qYXZh) | `0.00% <0.00%> (-73.13%)` | :arrow_down: |
   | [...che/pinot/controller/util/FileIngestionHelper.java](https://codecov.io/gh/apache/pinot/pull/8514/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci91dGlsL0ZpbGVJbmdlc3Rpb25IZWxwZXIuamF2YQ==) | `0.00% <ø> (-91.03%)` | :arrow_down: |
   | [.../org/apache/pinot/core/common/MinionConstants.java](https://codecov.io/gh/apache/pinot/pull/8514/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9jb21tb24vTWluaW9uQ29uc3RhbnRzLmphdmE=) | `0.00% <ø> (ø)` | |
   | [...t/core/operator/filter/CombinedFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/8514/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvQ29tYmluZWRGaWx0ZXJPcGVyYXRvci5qYXZh) | `0.00% <0.00%> (-70.00%)` | :arrow_down: |
   | [.../core/operator/filter/JsonMatchFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/8514/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvSnNvbk1hdGNoRmlsdGVyT3BlcmF0b3IuamF2YQ==) | `0.00% <0.00%> (-93.75%)` | :arrow_down: |
   | [.../core/operator/filter/TextMatchFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/8514/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvVGV4dE1hdGNoRmlsdGVyT3BlcmF0b3IuamF2YQ==) | `0.00% <0.00%> (-93.75%)` | :arrow_down: |
   | [...ache/pinot/plugin/minion/tasks/MergeTaskUtils.java](https://codecov.io/gh/apache/pinot/pull/8514/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvTWVyZ2VUYXNrVXRpbHMuamF2YQ==) | `47.27% <0.00%> (-48.89%)` | :arrow_down: |
   | [...on/tasks/mergerollup/MergeRollupTaskGenerator.java](https://codecov.io/gh/apache/pinot/pull/8514/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtcGx1Z2lucy9waW5vdC1taW5pb24tdGFza3MvcGlub3QtbWluaW9uLWJ1aWx0aW4tdGFza3Mvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3BsdWdpbi9taW5pb24vdGFza3MvbWVyZ2Vyb2xsdXAvTWVyZ2VSb2xsdXBUYXNrR2VuZXJhdG9yLmphdmE=) | `1.59% <0.00%> (-84.30%)` | :arrow_down: |
   | ... and [1246 more](https://codecov.io/gh/apache/pinot/pull/8514/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/8514?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/8514?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [44da1a0...fc36bdf](https://codecov.io/gh/apache/pinot/pull/8514?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] saurabhd336 commented on a diff in pull request #8514: Indicate what fields are getting ignored when adding configs

Posted by GitBox <gi...@apache.org>.
saurabhd336 commented on code in PR #8514:
URL: https://github.com/apache/pinot/pull/8514#discussion_r855765496


##########
pinot-spi/src/test/java/org/apache/pinot/spi/utils/TestClass.java:
##########
@@ -0,0 +1,84 @@
+/**
+ * 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.pinot.spi.utils;
+
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+
+
+@JsonIgnoreProperties(ignoreUnknown = true)
+public class TestClass {
+  int _primitiveIntegerField;
+  String _stringField;
+  Long _longField;
+  Klass _classField;

Review Comment:
   Ack



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #8514: Indicate what fields are getting ignored when adding configs

Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on code in PR #8514:
URL: https://github.com/apache/pinot/pull/8514#discussion_r859212236


##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/JsonUtils.java:
##########
@@ -71,12 +79,53 @@ private JsonUtils() {
   public static final ObjectReader DEFAULT_READER = DEFAULT_MAPPER.reader();
   public static final ObjectWriter DEFAULT_WRITER = DEFAULT_MAPPER.writer();
   public static final ObjectWriter DEFAULT_PRETTY_WRITER = DEFAULT_MAPPER.writerWithDefaultPrettyPrinter();
+  private static final TypeReference<HashMap<String, Object>> GENERIC_JSON_TYPE =
+      new TypeReference<HashMap<String, Object>>() { };
 
   public static <T> T stringToObject(String jsonString, Class<T> valueType)
       throws IOException {
     return DEFAULT_READER.forType(valueType).readValue(jsonString);
   }
 
+  public static <T> Pair<T, Map<String, Object>> stringToObjectAndUnparseableProps(String jsonString,

Review Comment:
   Suggest renaming
   ```suggestion
     public static <T> Pair<T, Map<String, Object>> stringToObjectAndUnrecognizedProperties(String jsonString,
   ```



##########
pinot-spi/src/test/java/org/apache/pinot/spi/utils/JsonUtilsTest.java:
##########
@@ -273,6 +274,20 @@ public void testFlatten()
     }
   }
 
+  @Test
+  public void testUnparseableJson() throws Exception {

Review Comment:
   Suggest renaming
   ```suggestion
     public void testUnrecognizedProperties() throws Exception {
   ```



##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java:
##########
@@ -153,16 +155,18 @@ public class PinotTableRestletResource {
   @Produces(MediaType.APPLICATION_JSON)
   @Path("/tables")
   @ApiOperation(value = "Adds a table", notes = "Adds a table")
-  public SuccessResponse addTable(
+  public ConfigSuccessResponse addTable(
       String tableConfigStr,
       @ApiParam(value = "comma separated list of validation type(s) to skip. supported types: (ALL|TASK|UPSERT)")
       @QueryParam("validationTypesToSkip") @Nullable String typesToSkip, @Context HttpHeaders httpHeaders,
       @Context Request request) {
     // TODO introduce a table config ctor with json string.
+    Pair<TableConfig, Map<String, Object>> tableConfigAndUnparsedProps;

Review Comment:
   Suggest renaming
   ```suggestion
       Pair<TableConfig, Map<String, Object>> tableConfigAndUnrecognizedProperties;
   ```



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] saurabhd336 commented on pull request #8514: Indicate what fields are getting ignored when adding configs (WIP)

Posted by GitBox <gi...@apache.org>.
saurabhd336 commented on PR #8514:
URL: https://github.com/apache/pinot/pull/8514#issuecomment-1096971037

   GH Issue: https://github.com/apache/pinot/issues/8318


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] npawar commented on a diff in pull request #8514: Indicate what fields are getting ignored when adding configs

Posted by GitBox <gi...@apache.org>.
npawar commented on code in PR #8514:
URL: https://github.com/apache/pinot/pull/8514#discussion_r855685792


##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java:
##########
@@ -492,18 +498,22 @@ public SuccessResponse updateTableConfig(
   @ApiOperation(value = "Validate table config for a table",
       notes = "This API returns the table config that matches the one you get from 'GET /tables/{tableName}'."
           + " This allows us to validate table config before apply.")
-  public String checkTableConfig(
+  public ObjectNode checkTableConfig(

Review Comment:
   should we take this opportunity to also wrap this into a ConfigValidationResponse object?



##########
pinot-spi/src/test/java/org/apache/pinot/spi/utils/TestClass.java:
##########
@@ -0,0 +1,84 @@
+/**
+ * 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.pinot.spi.utils;
+
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+
+
+@JsonIgnoreProperties(ignoreUnknown = true)
+public class TestClass {

Review Comment:
   should this just be inside JsonUtilsTest? It's very non-generic to be by itself 



##########
pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotTableRestletResourceTest.java:
##########
@@ -536,9 +552,9 @@ public void testValidate()
             .build();
 
     try {
-      ControllerTestUtils
-          .sendPostRequest(StringUtil.join("/", ControllerTestUtils.getControllerBaseApiUrl(), "tables", "validate"),
-              offlineTableConfig.toJsonString());
+      ControllerTestUtils.sendPostRequest(

Review Comment:
   would be nice to have couple of tests in validate. 
   1. validation success, but unparseable props
   2. validation failed 
   to make sure both these experiences are unaffected by this change
   (optional) test case in add and update too



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] saurabhd336 commented on a diff in pull request #8514: Indicate what fields are getting ignored when adding configs

Posted by GitBox <gi...@apache.org>.
saurabhd336 commented on code in PR #8514:
URL: https://github.com/apache/pinot/pull/8514#discussion_r855761860


##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java:
##########
@@ -492,18 +498,22 @@ public SuccessResponse updateTableConfig(
   @ApiOperation(value = "Validate table config for a table",
       notes = "This API returns the table config that matches the one you get from 'GET /tables/{tableName}'."
           + " This allows us to validate table config before apply.")
-  public String checkTableConfig(
+  public ObjectNode checkTableConfig(

Review Comment:
   I checked this. But it's getting a little tricky to do it in a backward compatible way. Since ConfigSuccessResponse extends SuccesResponse while this function returns a custom object..



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] saurabhd336 commented on a diff in pull request #8514: Indicate what fields are getting ignored when adding configs

Posted by GitBox <gi...@apache.org>.
saurabhd336 commented on code in PR #8514:
URL: https://github.com/apache/pinot/pull/8514#discussion_r857069606


##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/JsonUtils.java:
##########
@@ -71,12 +88,53 @@ private JsonUtils() {
   public static final ObjectReader DEFAULT_READER = DEFAULT_MAPPER.reader();
   public static final ObjectWriter DEFAULT_WRITER = DEFAULT_MAPPER.writer();
   public static final ObjectWriter DEFAULT_PRETTY_WRITER = DEFAULT_MAPPER.writerWithDefaultPrettyPrinter();
+  private static final TypeReference<HashMap<String, Object>> GENERIC_JSON_TYPE =
+      new TypeReference<HashMap<String, Object>>() { };
 
   public static <T> T stringToObject(String jsonString, Class<T> valueType)
       throws IOException {
     return DEFAULT_READER.forType(valueType).readValue(jsonString);
   }
 
+  public static <T> JsonPojoWithUnparsableProps<T> stringToObjectAndUnparseableProps(String jsonString, Class<T> klass)

Review Comment:
   Ack



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] saurabhd336 commented on a diff in pull request #8514: Indicate what fields are getting ignored when adding configs

Posted by GitBox <gi...@apache.org>.
saurabhd336 commented on code in PR #8514:
URL: https://github.com/apache/pinot/pull/8514#discussion_r855765360


##########
pinot-spi/src/test/java/org/apache/pinot/spi/utils/TestClass.java:
##########
@@ -0,0 +1,84 @@
+/**
+ * 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.pinot.spi.utils;
+
+import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
+
+
+@JsonIgnoreProperties(ignoreUnknown = true)
+public class TestClass {

Review Comment:
   Jackson is not able to work with inner classes. Hence had added it here. 
   Renamed to something more relevant



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] saurabhd336 commented on a diff in pull request #8514: Indicate what fields are getting ignored when adding configs

Posted by GitBox <gi...@apache.org>.
saurabhd336 commented on code in PR #8514:
URL: https://github.com/apache/pinot/pull/8514#discussion_r855764190


##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/JsonUtils.java:
##########
@@ -71,12 +88,53 @@ private JsonUtils() {
   public static final ObjectReader DEFAULT_READER = DEFAULT_MAPPER.reader();
   public static final ObjectWriter DEFAULT_WRITER = DEFAULT_MAPPER.writer();
   public static final ObjectWriter DEFAULT_PRETTY_WRITER = DEFAULT_MAPPER.writerWithDefaultPrettyPrinter();
+  private static final TypeReference<HashMap<String, Object>> GENERIC_JSON_TYPE =
+      new TypeReference<HashMap<String, Object>>() { };
 
   public static <T> T stringToObject(String jsonString, Class<T> valueType)
       throws IOException {
     return DEFAULT_READER.forType(valueType).readValue(jsonString);
   }
 
+  public static <T> JsonPojoWithUnparsableProps<T> stringToObjectAndUnparseableProps(String jsonString, Class<T> klass)

Review Comment:
   Ack on https://github.com/apache/pinot/pull/8514/files#r855450400
   [#2](https://github.com/apache/pinot/pull/8514/files#r855451277) The Pair class defined within the project expects 'T' to be extending Serializable, which would mean changing the base response classes. Other Pair implementations would need dependencies. Leaving it as it is for now.. Will revisit.



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] saurabhd336 commented on a diff in pull request #8514: Indicate what fields are getting ignored when adding configs

Posted by GitBox <gi...@apache.org>.
saurabhd336 commented on code in PR #8514:
URL: https://github.com/apache/pinot/pull/8514#discussion_r855783985


##########
pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotTableRestletResourceTest.java:
##########
@@ -536,9 +552,9 @@ public void testValidate()
             .build();
 
     try {
-      ControllerTestUtils
-          .sendPostRequest(StringUtil.join("/", ControllerTestUtils.getControllerBaseApiUrl(), "tables", "validate"),
-              offlineTableConfig.toJsonString());
+      ControllerTestUtils.sendPostRequest(

Review Comment:
   Added unrecognizedProperties tests for add, update and validate table APIs.
   Validation failed tests already exist for all APIs.



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


[GitHub] [pinot] saurabhd336 commented on a diff in pull request #8514: Indicate what fields are getting ignored when adding configs

Posted by GitBox <gi...@apache.org>.
saurabhd336 commented on code in PR #8514:
URL: https://github.com/apache/pinot/pull/8514#discussion_r859367289


##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java:
##########
@@ -153,16 +155,18 @@ public class PinotTableRestletResource {
   @Produces(MediaType.APPLICATION_JSON)
   @Path("/tables")
   @ApiOperation(value = "Adds a table", notes = "Adds a table")
-  public SuccessResponse addTable(
+  public ConfigSuccessResponse addTable(
       String tableConfigStr,
       @ApiParam(value = "comma separated list of validation type(s) to skip. supported types: (ALL|TASK|UPSERT)")
       @QueryParam("validationTypesToSkip") @Nullable String typesToSkip, @Context HttpHeaders httpHeaders,
       @Context Request request) {
     // TODO introduce a table config ctor with json string.
+    Pair<TableConfig, Map<String, Object>> tableConfigAndUnparsedProps;

Review Comment:
   Ack



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org