yuanlihan commented on code in PR #14984:
URL: https://github.com/apache/druid/pull/14984#discussion_r1338156758
##########
extensions-core/hdfs-storage/src/main/java/org/apache/druid/storage/hdfs/HdfsDataSegmentPusher.java:
##########
@@ -124,8 +124,18 @@ public DataSegment pushToPath(File inDir, DataSegment
segment, String storageDir
fullyQualifiedStorageDirectory.get(),
storageDirSuffix
);
-
- final String storageDir = StringUtils.format("%s/%s",
fullyQualifiedStorageDirectory.get(), storageDirSuffix);
+
+ final String outIndexFilePath;
+ if (storageDirSuffix.endsWith("index.zip")) {
Review Comment:
@abhishekagarwal87 Thanks for your comment!
> The function name is a bit confusing since storageDirSuffix is really a
suffix to the filename.
I think `storageDirSuffix` shouldn't be the suffix to the filename but
instead as the prefix dir of file `index.zip`.
I have concern that adding a third method will need to touch all
implementations. I think it's ok to reuse `pushToPath` method inside `push`
method as long as it complies with passing a storageDirSuffix not file path to
method `pushToPath`.
Currently, only two implementations, `HdfsDataSegmentPusher` and
`AzureDataSegmentPusher`, violate to pass file path to their method
`pushToPath` methods. Other implementations only format `storageDirSuffix` and
reuse method `pushToPath` to format file path with `index.zip` file.
We can also enhance the method doc of `pushToPath`
https://github.com/apache/druid/blob/3ae5e978012e7a553e6ad3007eb210ab8da5b6fc/processing/src/main/java/org/apache/druid/segment/loading/DataSegmentPusher.java#L70-L75
--
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]
