jihoonson commented on a change in pull request #10278:
URL: https://github.com/apache/druid/pull/10278#discussion_r471866557



##########
File path: 
indexing-service/src/main/java/org/apache/druid/indexing/common/task/AbstractTask.java
##########
@@ -175,23 +162,6 @@ public String toString()
            '}';
   }
 
-  /**
-   * Start helper methods
-   *
-   * @param objects objects to join
-   *
-   * @return string of joined objects
-   */
-  static String joinId(List<Object> objects)
-  {
-    return ID_JOINER.join(objects);

Review comment:
       Thanks, removed.

##########
File path: 
server/src/main/java/org/apache/druid/client/indexing/IndexingServiceClient.java
##########
@@ -31,11 +31,27 @@
 
 public interface IndexingServiceClient
 {
-  void killUnusedSegments(String dataSource, Interval interval);
+  default void killUnusedSegments(String dataSource, Interval interval)
+  {
+    killUnusedSegments("", dataSource, interval);

Review comment:
       Oh yeah, we don't need those new default methods here. I'm not sure why 
I added them :sweat_smile:. `IndexingServiceClient` is not a [public 
API](https://github.com/apache/druid/blob/master/core/src/main/java/org/apache/druid/guice/annotations/PublicApi.java).
 We don't support backward compatibility for those APIs.

##########
File path: 
server/src/main/java/org/apache/druid/client/indexing/ClientCompactionTaskQuery.java
##########
@@ -43,14 +42,14 @@
 
   @JsonCreator
   public ClientCompactionTaskQuery(
-      @JsonProperty("id") @Nullable String id,
+      @JsonProperty("id") String id,
       @JsonProperty("dataSource") String dataSource,
       @JsonProperty("ioConfig") ClientCompactionIOConfig ioConfig,
       @JsonProperty("tuningConfig") ClientCompactionTaskQueryTuningConfig 
tuningConfig,
       @JsonProperty("context") Map<String, Object> context
   )
   {
-    this.id = id == null ? IdUtils.newTaskId(TYPE, dataSource, null) : id;
+    this.id = Preconditions.checkNotNull(id, "id");

Review comment:
       Seems like you were seeing changes between commits. The `id` field was 
newly added in this PR. Before this PR, `id` was initialized when 
`ClientCompactionTaskQuery` is converted into `CompactionTask`. Since the `id` 
field in `ClientCompactionTaskQuery` was never accessed on the coordinator 
side, I think there should be no backward-compatibility issue.




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



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to