chenboat commented on a change in pull request #5221: Add a new server api for download of segments. URL: https://github.com/apache/incubator-pinot/pull/5221#discussion_r408487929
########## File path: pinot-server/src/test/java/org/apache/pinot/server/api/BaseResourceTest.java ########## @@ -58,8 +59,8 @@ protected static final String TABLE_NAME = "testTable"; private final Map<String, TableDataManager> _tableDataManagerMap = new HashMap<>(); - protected final List<ImmutableSegment> _indexSegments = new ArrayList<>(); - + protected final List<ImmutableSegment> _realtimeIndexSegments = new ArrayList<>(); Review comment: Before we talk about the separation of two kinds of segments, I want to point out this test was set up almost like a mini integration tests with mock servers and download clients. So ideally it should reflect a true Pinot server setup environment with proper directory naming associated with table name and type -- this was NOT done earlier i think (e.g., the test table name does not have type). The modified test code adheres to these expectations. The separation of segments also follows the same idea: they are just different segments. For your comments about realtime index segment, I believe you are referring to the fact that CONSUMING segment could not be downloaded just like COMPLETED segment, right? Yes, that is true for now and we can add a note to API as well. . ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
