[GitHub] [pinot] mcvsubbu commented on a change in pull request #7908: Add broker validation for hybrid tableConfig creation

[GitBox <gi...@apache.org>]

mcvsubbu commented on a change in pull request #7908:
URL: https://github.com/apache/pinot/pull/7908#discussion_r769897139



##########
File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java
##########
@@ -827,5 +827,20 @@ public static void verifyHybridTableConfigs(String rawTableName, TableConfig off
           "Time column names are different for table: %s! Offline time column name: %s. Realtime time column name: %s",
           rawTableName, offlineTimeColumnName, realtimeTimeColumnName));
     }
+    TenantConfig offlineTenantConfig = offlineTableConfig.getTenantConfig();
+    TenantConfig realtimeTenantConfig = realtimeTableConfig.getTenantConfig();
+    String offlineBroker = offlineTenantConfig.getBroker();
+    String realtimeBroker = realtimeTenantConfig.getBroker();
+    if (offlineBroker == null || realtimeBroker == null) {
+      throw new IllegalStateException(String.format(

Review comment:
       I hope these get translated to 4xx error and not 5xx. I would think of IllegalStateException as something that is wrong on the server end. These are clearly bad user inputs causing this exception, so it should be IllegalArgumentException ? I understand you are following the pattern from before, but if you could dig on this a bit and fix it that will be awesome.
   
   Thanks.




-- 
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