FrankChen021 commented on code in PR #20806:
URL: https://github.com/apache/druid/pull/20806#discussion_r4438137838
##########
processing/src/main/java/org/apache/druid/segment/loading/DeepStorageSegmentConfig.java:
##########
@@ -40,34 +43,49 @@
*/
public class DeepStorageSegmentConfig
{
- @JsonProperty
+ private static final Logger log = new Logger(DeepStorageSegmentConfig.class);
+
+ @JsonProperty("compressionFormat")
@Nullable
- private final Boolean zip;
+ private final CompressionUtils.Format compressionFormat;
@JsonCreator
- public DeepStorageSegmentConfig(@JsonProperty("zip") @Nullable Boolean zip)
+ public DeepStorageSegmentConfig(
Review Comment:
[P1] Update the remaining Boolean constructor call site
**Finding:** Removing the one-argument Boolean constructor leaves
indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/SeekableStreamIndexTaskTestBase.java:679
calling new DeepStorageSegmentConfig(true). That source no longer matches
either remaining constructor, so the indexing-service test sources cannot
compile on this head.
**Suggestion:** Update that caller to the retained two-argument form or keep
a deprecated compatibility constructor, and search all modules for the removed
overload before merging.
##########
extensions-core/hdfs-storage/src/main/java/org/apache/druid/storage/hdfs/HdfsDataSegmentPusher.java:
##########
@@ -70,19 +79,43 @@ public HdfsDataSegmentPusher(
@Hdfs Configuration hadoopConfig
)
{
- this(config, hadoopConfig, null);
+ this(config, hadoopConfig, null, new DeepStorageSegmentConfig());
+ }
+
+ public HdfsDataSegmentPusher(
Review Comment:
[P2] Avoid an ambiguous public constructor overload
**Finding:** The new three-argument DeepStorageSegmentConfig overload sits
beside the existing three-argument ServiceEmitter overload. Existing source
calls such as new HdfsDataSegmentPusher(config, hadoopConfig, null), which
previously selected ServiceEmitter, now fail with an ambiguous-constructor
compile error.
**Suggestion:** Remove this convenience overload or replace it with a
differently named factory or constructor so the retained ServiceEmitter
signature remains source-compatible for null arguments.
--
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]
