shounakmk219 commented on code in PR #14503:
URL: https://github.com/apache/pinot/pull/14503#discussion_r1855876987
##########
pinot-controller/src/test/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResourceTest.java:
##########
@@ -56,20 +66,27 @@ public class PinotSegmentUploadDownloadRestletResourceTest {
private static final String TABLE_NAME = "table_abc";
private static final String SEGMENT_NAME = "segment_xyz";
+ private static final String HOST = "localhost";
+ private static final String PORT = "12345";
+ private static final File DATA_DIR =
+ new File(FileUtils.getTempDirectory(),
"PinotSegmentUploadDownloadRestletResourceTest");
+ private static final File LOCAL_TEMP_DIR = new File(DATA_DIR, "localTemp");
Review Comment:
Delete these directories in `tearDown()` or see if you can use `_tempDir `
itself
##########
pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/ControllerFilePathProvider.java:
##########
@@ -126,14 +128,38 @@ public URI getDataDirURI() {
}
public File getFileUploadTempDir() {
+ if (!Files.exists(_fileUploadTempDir.toPath())) {
Review Comment:
Extract this logic into a method to avoid replicating it.
##########
pinot-controller/src/test/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResourceTest.java:
##########
@@ -158,7 +175,8 @@ public void testEncryptSegmentIfNeededNoEncryption() {
}
@Test
- public void testCreateSegmentFileFromBodyPart() throws IOException {
+ public void testCreateSegmentFileFromBodyPart()
Review Comment:
nit: revert the formatting
##########
pinot-controller/src/test/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResourceTest.java:
##########
@@ -249,4 +267,58 @@ public void testValidateMultiPartForBatchSegmentUpload() {
// validate – should not throw exception
PinotSegmentUploadDownloadRestletResource.validateMultiPartForBatchSegmentUpload(bodyParts);
}
+
+ @Test
+ public void testUploadSegmentWithMissingTmpDir()
Review Comment:
I don't see this test adding anymore value than the one created in
`ControllerFilePathProviderTest `, maybe you can add an integration test in
`SegmentUploadIntegrationTest` to validate the segment upload flow?
##########
pinot-controller/src/test/java/org/apache/pinot/controller/api/resources/PinotSegmentUploadDownloadRestletResourceTest.java:
##########
@@ -56,20 +66,27 @@ public class PinotSegmentUploadDownloadRestletResourceTest {
private static final String TABLE_NAME = "table_abc";
private static final String SEGMENT_NAME = "segment_xyz";
+ private static final String HOST = "localhost";
+ private static final String PORT = "12345";
+ private static final File DATA_DIR =
+ new File(FileUtils.getTempDirectory(),
"PinotSegmentUploadDownloadRestletResourceTest");
+ private static final File LOCAL_TEMP_DIR = new File(DATA_DIR, "localTemp");
private PinotSegmentUploadDownloadRestletResource _resource = new
PinotSegmentUploadDownloadRestletResource();
private File _encryptedFile;
private File _decryptedFile;
private File _tempDir;
@BeforeMethod
- public void setUp() throws IOException {
+ public void setUp()
+ throws IOException {
_tempDir = new File(FileUtils.getTempDirectory(), "test-" +
UUID.randomUUID());
FileUtils.forceMkdir(_tempDir);
}
@AfterMethod
- public void tearDown() throws IOException {
+ public void tearDown()
+ throws IOException {
Review Comment:
nit: revert the formatting
--
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-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
