kfaraz commented on code in PR #18341:
URL: https://github.com/apache/druid/pull/18341#discussion_r2262768679
##########
extensions-core/kubernetes-overlord-extensions/src/test/java/org/apache/druid/k8s/overlord/KubernetesPeonLifecycleTest.java:
##########
@@ -339,7 +339,6 @@ public void test_join_withoutJob_returnsFailedTaskStatus()
throws IOException
EasyMock.expectLastCall().once();
stateListener.stateChanged(KubernetesPeonLifecycle.State.STOPPED, ID);
EasyMock.expectLastCall().once();
-
Review Comment:
The changes to this file seem unintentional. Please revert these.
##########
indexing-service/src/main/java/org/apache/druid/guice/IndexingServiceTaskLogsModule.java:
##########
@@ -20,32 +20,94 @@
package org.apache.druid.guice;
import com.google.inject.Binder;
+import com.google.inject.Inject;
import com.google.inject.Key;
import com.google.inject.Module;
import com.google.inject.multibindings.MapBinder;
+import com.google.inject.name.Names;
import org.apache.druid.indexing.common.config.FileTaskLogsConfig;
import org.apache.druid.indexing.common.tasklogs.FileTaskLogs;
+import org.apache.druid.indexing.common.tasklogs.SwitchingTaskLogs;
import org.apache.druid.tasklogs.NoopTaskLogs;
import org.apache.druid.tasklogs.TaskLogKiller;
import org.apache.druid.tasklogs.TaskLogPusher;
import org.apache.druid.tasklogs.TaskLogs;
import org.apache.druid.tasklogs.TaskPayloadManager;
+import java.util.Properties;
+
/**
+ *
*/
public class IndexingServiceTaskLogsModule implements Module
{
+ private Properties props;
+
+ @Inject
+ public IndexingServiceTaskLogsModule(Properties props)
Review Comment:
This will help you retain the zero arg constructor.
```suggestion
public setProperties(Properties props)
```
##########
processing/src/main/java/org/apache/druid/guice/Binders.java:
##########
@@ -59,4 +60,16 @@ public static MapBinder<String, TaskLogs>
taskLogsBinder(Binder binder)
{
return PolyBind.optionBinder(binder, Key.get(TaskLogs.class));
}
+
+ /**
+ * Binds implementation of {@link TaskLogs} to the named keys for injection.
+ */
+ public static <T extends TaskLogs> void bindTaskLogs(Binder binder, String
type, Class<T> clazz)
+ {
+ PolyBind.optionBinder(binder,
Key.get(TaskLogs.class)).addBinding(type).to(clazz);
+ PolyBind.optionBinder(binder, Key.get(TaskLogs.class,
Names.named("switching.reportType"))).addBinding(type).to(clazz);
+ PolyBind.optionBinder(binder, Key.get(TaskLogs.class,
Names.named("switching.streamType"))).addBinding(type).to(clazz);
+ PolyBind.optionBinder(binder, Key.get(TaskLogs.class,
Names.named("switching.pushType"))).addBinding(type).to(clazz);
+ PolyBind.optionBinder(binder, Key.get(TaskLogs.class,
Names.named("switching.defaultType"))).addBinding(type).to(clazz);
Review Comment:
Please use the constants from `SwitchingTaskLogs`.
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/tasklogs/SwitchingTaskLogs.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.indexing.common.tasklogs;
+
+import com.google.common.base.Optional;
+import com.google.inject.Inject;
+import com.google.inject.name.Named;
+import org.apache.druid.tasklogs.TaskLogs;
+
+import javax.annotation.Nullable;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+
+public class SwitchingTaskLogs implements TaskLogs
+{
+
+ public static final String PROPERTY_PREFIX_SWITCHING =
"druid.indexer.logs.switching";
+ public static final String PROPERTY_KEY_SWITCHING_DEFAULT_TYPE =
PROPERTY_PREFIX_SWITCHING + ".defaultType";
+ public static final String PROPERTY_KEY_SWITCHING_PUSH_TYPE =
PROPERTY_PREFIX_SWITCHING + ".pushType";
+ public static final String PROPERTY_KEY_SWITCHING_STREAM_TYPE =
PROPERTY_PREFIX_SWITCHING + ".streamType";
+ public static final String PROPERTY_KEY_SWITCHING_REPORTS_TYPE =
PROPERTY_PREFIX_SWITCHING + ".reportsType";
+
+ public static final String KEY_SWITCHING_REPORTS = "switching.reportType";
+ public static final String KEY_SWITCHING_STREAM = "switching.streamType";
+ public static final String KEY_SWITCHING_PUSH = "switching.pushType";
Review Comment:
`pushType` seems ambiguous, use `logPushType` instead.
##########
processing/src/main/java/org/apache/druid/guice/Binders.java:
##########
@@ -59,4 +60,16 @@ public static MapBinder<String, TaskLogs>
taskLogsBinder(Binder binder)
{
return PolyBind.optionBinder(binder, Key.get(TaskLogs.class));
}
+
+ /**
+ * Binds implementation of {@link TaskLogs} to the named keys for injection.
+ */
+ public static <T extends TaskLogs> void bindTaskLogs(Binder binder, String
type, Class<T> clazz)
Review Comment:
```suggestion
public static <T extends TaskLogs> void bindTaskLogs(Binder binder, String
type, Class<T> taskLogsType)
```
##########
indexing-service/src/main/java/org/apache/druid/guice/IndexingServiceTaskLogsModule.java:
##########
@@ -20,32 +20,94 @@
package org.apache.druid.guice;
import com.google.inject.Binder;
+import com.google.inject.Inject;
import com.google.inject.Key;
import com.google.inject.Module;
import com.google.inject.multibindings.MapBinder;
+import com.google.inject.name.Names;
import org.apache.druid.indexing.common.config.FileTaskLogsConfig;
import org.apache.druid.indexing.common.tasklogs.FileTaskLogs;
+import org.apache.druid.indexing.common.tasklogs.SwitchingTaskLogs;
import org.apache.druid.tasklogs.NoopTaskLogs;
import org.apache.druid.tasklogs.TaskLogKiller;
import org.apache.druid.tasklogs.TaskLogPusher;
import org.apache.druid.tasklogs.TaskLogs;
import org.apache.druid.tasklogs.TaskPayloadManager;
+import java.util.Properties;
+
/**
+ *
*/
public class IndexingServiceTaskLogsModule implements Module
{
+ private Properties props;
+
+ @Inject
+ public IndexingServiceTaskLogsModule(Properties props)
+ {
+ this.props = props;
+ }
+
@Override
public void configure(Binder binder)
{
PolyBind.createChoice(binder, "druid.indexer.logs.type",
Key.get(TaskLogs.class), Key.get(FileTaskLogs.class));
+ PolyBind.createChoice(
+ binder,
+ SwitchingTaskLogs.PROPERTY_KEY_SWITCHING_DEFAULT_TYPE,
+ Key.get(TaskLogs.class,
Names.named(SwitchingTaskLogs.KEY_SWITCHING_DEFAULT)),
+ Key.get(FileTaskLogs.class)
+ );
+
+ if (props != null &&
props.getProperty(SwitchingTaskLogs.PROPERTY_KEY_SWITCHING_PUSH_TYPE) != null) {
+ PolyBind.createChoice(
+ binder,
+ SwitchingTaskLogs.PROPERTY_KEY_SWITCHING_PUSH_TYPE,
+ Key.get(TaskLogs.class,
Names.named(SwitchingTaskLogs.KEY_SWITCHING_PUSH)),
+ null
+ );
+ } else {
+ binder.bind(Key.get(TaskLogs.class,
Names.named(SwitchingTaskLogs.KEY_SWITCHING_PUSH)))
+ .toProvider(() -> null);
+ }
+
+ if (props != null &&
props.getProperty(SwitchingTaskLogs.PROPERTY_KEY_SWITCHING_REPORTS_TYPE) !=
null) {
+ PolyBind.createChoice(
+ binder,
+ SwitchingTaskLogs.PROPERTY_KEY_SWITCHING_REPORTS_TYPE,
+ Key.get(TaskLogs.class,
Names.named(SwitchingTaskLogs.KEY_SWITCHING_REPORTS)),
+ null
+ );
+ } else {
+ binder.bind(Key.get(TaskLogs.class,
Names.named(SwitchingTaskLogs.KEY_SWITCHING_REPORTS)))
+ .toProvider(() -> null);
+ }
+
+ if (props != null &&
props.getProperty(SwitchingTaskLogs.PROPERTY_KEY_SWITCHING_STREAM_TYPE) !=
null) {
+ PolyBind.createChoice(
+ binder,
+ SwitchingTaskLogs.PROPERTY_KEY_SWITCHING_STREAM_TYPE,
+ Key.get(TaskLogs.class,
Names.named(SwitchingTaskLogs.KEY_SWITCHING_STREAM)),
+ null
+ );
+ } else {
+ binder.bind(Key.get(TaskLogs.class,
Names.named(SwitchingTaskLogs.KEY_SWITCHING_STREAM)))
+ .toProvider(() -> null);
+ }
Review Comment:
Please put this logic in a method so that it can be reused for the 3 cases.
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/tasklogs/SwitchingTaskLogs.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.indexing.common.tasklogs;
+
+import com.google.common.base.Optional;
+import com.google.inject.Inject;
+import com.google.inject.name.Named;
+import org.apache.druid.tasklogs.TaskLogs;
+
+import javax.annotation.Nullable;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+
+public class SwitchingTaskLogs implements TaskLogs
Review Comment:
Please add a javadoc.
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/tasklogs/SwitchingTaskLogs.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.indexing.common.tasklogs;
+
+import com.google.common.base.Optional;
+import com.google.inject.Inject;
+import com.google.inject.name.Named;
+import org.apache.druid.tasklogs.TaskLogs;
+
+import javax.annotation.Nullable;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+
+public class SwitchingTaskLogs implements TaskLogs
+{
+
+ public static final String PROPERTY_PREFIX_SWITCHING =
"druid.indexer.logs.switching";
Review Comment:
Please remove the word `SWITCHING` from all the constants since they already
reside in the `SwitchingTaskLogs` class.
```suggestion
public static final String PROPERTY_PREFIX =
"druid.indexer.logs.switching";
```
##########
indexing-service/src/test/java/org/apache/druid/guice/IndexingServiceTaskLogsModuleTest.java:
##########
@@ -0,0 +1,142 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.guice;
+
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.google.inject.Key;
+import com.google.inject.Scopes;
+import com.google.inject.name.Names;
+import org.apache.druid.indexing.common.tasklogs.FileTaskLogs;
+import org.apache.druid.indexing.common.tasklogs.SwitchingTaskLogs;
+import org.apache.druid.jackson.JacksonModule;
+import org.apache.druid.tasklogs.NoopTaskLogs;
+import org.apache.druid.tasklogs.TaskLogs;
+import org.junit.Assert;
+import org.junit.Test;
+
+import javax.validation.ConstraintViolation;
+import javax.validation.Validator;
+import javax.validation.executable.ExecutableValidator;
+import javax.validation.metadata.BeanDescriptor;
+import java.util.Collections;
+import java.util.Properties;
+import java.util.Set;
+
+public class IndexingServiceTaskLogsModuleTest
+{
+
+ @Test
+ public void test_switchingTaskLogs_shouldUseDefaultWhenNoPusherConfigured()
+ {
+ Properties props = new Properties();
+ props.setProperty("druid.indexer.logs.type", "switching");
+ props.setProperty("druid.indexer.logs.switching.defaultType", "file");
+
+ Injector injector = Guice.createInjector(
+ binder -> {
+ binder.bindScope(LazySingleton.class, Scopes.SINGLETON);
+ binder.bind(Properties.class).toInstance(props);
+ binder.bind(Validator.class).toInstance(new MockValidator());
+ },
+ new IndexingServiceTaskLogsModule(props),
+ new JacksonModule()
+ );
+
+ TaskLogs taskLogs = injector.getInstance(TaskLogs.class);
+ Assert.assertTrue(taskLogs instanceof SwitchingTaskLogs);
+ }
+
+ @Test
+ public void test_providePusher_shouldReturnConfiguredImplementaions()
+ {
+ Properties props = new Properties();
+ props.setProperty("druid.indexer.logs.type", "switching");
+ props.setProperty("druid.indexer.logs.switching.defaultType", "noop");
+ props.setProperty("druid.indexer.logs.switching.pushType", "file");
+ props.setProperty("druid.indexer.logs.switching.reportsType", "noop");
+ props.setProperty("druid.indexer.logs.switching.streamType", "file");
+
+ Injector injector = Guice.createInjector(
+ binder -> {
+ binder.bindScope(LazySingleton.class, Scopes.SINGLETON);
+ binder.bind(Properties.class).toInstance(props);
+ binder.bind(Validator.class).toInstance(new MockValidator());
+ },
+ new IndexingServiceTaskLogsModule(props),
+ new JacksonModule()
+ );
Review Comment:
This can be put in a method for reuse in all the tests.
##########
indexing-service/src/test/java/org/apache/druid/guice/IndexingServiceTaskLogsModuleTest.java:
##########
@@ -0,0 +1,142 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.guice;
+
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.google.inject.Key;
+import com.google.inject.Scopes;
+import com.google.inject.name.Names;
+import org.apache.druid.indexing.common.tasklogs.FileTaskLogs;
+import org.apache.druid.indexing.common.tasklogs.SwitchingTaskLogs;
+import org.apache.druid.jackson.JacksonModule;
+import org.apache.druid.tasklogs.NoopTaskLogs;
+import org.apache.druid.tasklogs.TaskLogs;
+import org.junit.Assert;
+import org.junit.Test;
+
+import javax.validation.ConstraintViolation;
+import javax.validation.Validator;
+import javax.validation.executable.ExecutableValidator;
+import javax.validation.metadata.BeanDescriptor;
+import java.util.Collections;
+import java.util.Properties;
+import java.util.Set;
+
+public class IndexingServiceTaskLogsModuleTest
+{
+
+ @Test
+ public void test_switchingTaskLogs_shouldUseDefaultWhenNoPusherConfigured()
+ {
+ Properties props = new Properties();
+ props.setProperty("druid.indexer.logs.type", "switching");
+ props.setProperty("druid.indexer.logs.switching.defaultType", "file");
+
+ Injector injector = Guice.createInjector(
+ binder -> {
+ binder.bindScope(LazySingleton.class, Scopes.SINGLETON);
+ binder.bind(Properties.class).toInstance(props);
+ binder.bind(Validator.class).toInstance(new MockValidator());
Review Comment:
Why is this needed?
##########
indexing-service/src/test/java/org/apache/druid/guice/IndexingServiceTaskLogsModuleTest.java:
##########
@@ -0,0 +1,142 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.guice;
+
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.google.inject.Key;
+import com.google.inject.Scopes;
+import com.google.inject.name.Names;
+import org.apache.druid.indexing.common.tasklogs.FileTaskLogs;
+import org.apache.druid.indexing.common.tasklogs.SwitchingTaskLogs;
+import org.apache.druid.jackson.JacksonModule;
+import org.apache.druid.tasklogs.NoopTaskLogs;
+import org.apache.druid.tasklogs.TaskLogs;
+import org.junit.Assert;
+import org.junit.Test;
+
+import javax.validation.ConstraintViolation;
+import javax.validation.Validator;
+import javax.validation.executable.ExecutableValidator;
+import javax.validation.metadata.BeanDescriptor;
+import java.util.Collections;
+import java.util.Properties;
+import java.util.Set;
+
+public class IndexingServiceTaskLogsModuleTest
+{
+
+ @Test
+ public void test_switchingTaskLogs_shouldUseDefaultWhenNoPusherConfigured()
+ {
+ Properties props = new Properties();
+ props.setProperty("druid.indexer.logs.type", "switching");
+ props.setProperty("druid.indexer.logs.switching.defaultType", "file");
+
+ Injector injector = Guice.createInjector(
+ binder -> {
+ binder.bindScope(LazySingleton.class, Scopes.SINGLETON);
+ binder.bind(Properties.class).toInstance(props);
+ binder.bind(Validator.class).toInstance(new MockValidator());
+ },
+ new IndexingServiceTaskLogsModule(props),
+ new JacksonModule()
+ );
+
+ TaskLogs taskLogs = injector.getInstance(TaskLogs.class);
+ Assert.assertTrue(taskLogs instanceof SwitchingTaskLogs);
+ }
+
+ @Test
+ public void test_providePusher_shouldReturnConfiguredImplementaions()
Review Comment:
```suggestion
public void test_switchingTaskLogs_usesConfiguredPusherType()
```
##########
indexing-service/src/test/java/org/apache/druid/guice/IndexingServiceTaskLogsModuleTest.java:
##########
@@ -0,0 +1,142 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.guice;
+
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.google.inject.Key;
+import com.google.inject.Scopes;
+import com.google.inject.name.Names;
+import org.apache.druid.indexing.common.tasklogs.FileTaskLogs;
+import org.apache.druid.indexing.common.tasklogs.SwitchingTaskLogs;
+import org.apache.druid.jackson.JacksonModule;
+import org.apache.druid.tasklogs.NoopTaskLogs;
+import org.apache.druid.tasklogs.TaskLogs;
+import org.junit.Assert;
+import org.junit.Test;
+
+import javax.validation.ConstraintViolation;
+import javax.validation.Validator;
+import javax.validation.executable.ExecutableValidator;
+import javax.validation.metadata.BeanDescriptor;
+import java.util.Collections;
+import java.util.Properties;
+import java.util.Set;
+
+public class IndexingServiceTaskLogsModuleTest
+{
+
+ @Test
+ public void test_switchingTaskLogs_shouldUseDefaultWhenNoPusherConfigured()
+ {
+ Properties props = new Properties();
+ props.setProperty("druid.indexer.logs.type", "switching");
+ props.setProperty("druid.indexer.logs.switching.defaultType", "file");
+
+ Injector injector = Guice.createInjector(
+ binder -> {
+ binder.bindScope(LazySingleton.class, Scopes.SINGLETON);
+ binder.bind(Properties.class).toInstance(props);
+ binder.bind(Validator.class).toInstance(new MockValidator());
+ },
+ new IndexingServiceTaskLogsModule(props),
+ new JacksonModule()
+ );
+
+ TaskLogs taskLogs = injector.getInstance(TaskLogs.class);
+ Assert.assertTrue(taskLogs instanceof SwitchingTaskLogs);
Review Comment:
We should verify that the named types are being defaulted to `file`.
##########
processing/src/test/java/org/apache/druid/guice/BindersTest.java:
##########
@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.guice;
+
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.google.inject.Key;
+import com.google.inject.name.Names;
+import org.apache.druid.tasklogs.NoopTaskLogs;
+import org.apache.druid.tasklogs.TaskLogs;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import java.util.Properties;
+
+class BindersTest
Review Comment:
Seems cleaner:
```suggestion
public class BindersTest
```
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/tasklogs/SwitchingTaskLogs.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.indexing.common.tasklogs;
+
+import com.google.common.base.Optional;
+import com.google.inject.Inject;
+import com.google.inject.name.Named;
+import org.apache.druid.tasklogs.TaskLogs;
+
+import javax.annotation.Nullable;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+
+public class SwitchingTaskLogs implements TaskLogs
+{
+
+ public static final String PROPERTY_PREFIX_SWITCHING =
"druid.indexer.logs.switching";
+ public static final String PROPERTY_KEY_SWITCHING_DEFAULT_TYPE =
PROPERTY_PREFIX_SWITCHING + ".defaultType";
+ public static final String PROPERTY_KEY_SWITCHING_PUSH_TYPE =
PROPERTY_PREFIX_SWITCHING + ".pushType";
Review Comment:
```suggestion
public static final String PROPERTY_KEY_SWITCHING_PUSH_TYPE =
PROPERTY_PREFIX_SWITCHING + ".logPushType";
```
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/tasklogs/SwitchingTaskLogs.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.indexing.common.tasklogs;
+
+import com.google.common.base.Optional;
+import com.google.inject.Inject;
+import com.google.inject.name.Named;
+import org.apache.druid.tasklogs.TaskLogs;
+
+import javax.annotation.Nullable;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+
+public class SwitchingTaskLogs implements TaskLogs
+{
+
+ public static final String PROPERTY_PREFIX_SWITCHING =
"druid.indexer.logs.switching";
+ public static final String PROPERTY_KEY_SWITCHING_DEFAULT_TYPE =
PROPERTY_PREFIX_SWITCHING + ".defaultType";
+ public static final String PROPERTY_KEY_SWITCHING_PUSH_TYPE =
PROPERTY_PREFIX_SWITCHING + ".pushType";
+ public static final String PROPERTY_KEY_SWITCHING_STREAM_TYPE =
PROPERTY_PREFIX_SWITCHING + ".streamType";
Review Comment:
```suggestion
public static final String PROPERTY_KEY_SWITCHING_STREAM_TYPE =
PROPERTY_PREFIX_SWITCHING + ".logStreamType";
```
##########
indexing-service/src/main/java/org/apache/druid/guice/IndexingServiceTaskLogsModule.java:
##########
@@ -20,32 +20,94 @@
package org.apache.druid.guice;
import com.google.inject.Binder;
+import com.google.inject.Inject;
import com.google.inject.Key;
import com.google.inject.Module;
import com.google.inject.multibindings.MapBinder;
+import com.google.inject.name.Names;
import org.apache.druid.indexing.common.config.FileTaskLogsConfig;
import org.apache.druid.indexing.common.tasklogs.FileTaskLogs;
+import org.apache.druid.indexing.common.tasklogs.SwitchingTaskLogs;
import org.apache.druid.tasklogs.NoopTaskLogs;
import org.apache.druid.tasklogs.TaskLogKiller;
import org.apache.druid.tasklogs.TaskLogPusher;
import org.apache.druid.tasklogs.TaskLogs;
import org.apache.druid.tasklogs.TaskPayloadManager;
+import java.util.Properties;
+
/**
+ *
*/
public class IndexingServiceTaskLogsModule implements Module
{
+ private Properties props;
+
+ @Inject
+ public IndexingServiceTaskLogsModule(Properties props)
+ {
+ this.props = props;
+ }
+
@Override
public void configure(Binder binder)
{
PolyBind.createChoice(binder, "druid.indexer.logs.type",
Key.get(TaskLogs.class), Key.get(FileTaskLogs.class));
+ PolyBind.createChoice(
+ binder,
+ SwitchingTaskLogs.PROPERTY_KEY_SWITCHING_DEFAULT_TYPE,
+ Key.get(TaskLogs.class,
Names.named(SwitchingTaskLogs.KEY_SWITCHING_DEFAULT)),
+ Key.get(FileTaskLogs.class)
+ );
+
+ if (props != null &&
props.getProperty(SwitchingTaskLogs.PROPERTY_KEY_SWITCHING_PUSH_TYPE) != null) {
+ PolyBind.createChoice(
+ binder,
+ SwitchingTaskLogs.PROPERTY_KEY_SWITCHING_PUSH_TYPE,
+ Key.get(TaskLogs.class,
Names.named(SwitchingTaskLogs.KEY_SWITCHING_PUSH)),
+ null
+ );
+ } else {
+ binder.bind(Key.get(TaskLogs.class,
Names.named(SwitchingTaskLogs.KEY_SWITCHING_PUSH)))
+ .toProvider(() -> null);
+ }
+
+ if (props != null &&
props.getProperty(SwitchingTaskLogs.PROPERTY_KEY_SWITCHING_REPORTS_TYPE) !=
null) {
+ PolyBind.createChoice(
+ binder,
+ SwitchingTaskLogs.PROPERTY_KEY_SWITCHING_REPORTS_TYPE,
+ Key.get(TaskLogs.class,
Names.named(SwitchingTaskLogs.KEY_SWITCHING_REPORTS)),
+ null
+ );
+ } else {
+ binder.bind(Key.get(TaskLogs.class,
Names.named(SwitchingTaskLogs.KEY_SWITCHING_REPORTS)))
+ .toProvider(() -> null);
+ }
+
+ if (props != null &&
props.getProperty(SwitchingTaskLogs.PROPERTY_KEY_SWITCHING_STREAM_TYPE) !=
null) {
Review Comment:
`props` is never going to be null.
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/tasklogs/SwitchingTaskLogs.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.indexing.common.tasklogs;
+
+import com.google.common.base.Optional;
+import com.google.inject.Inject;
+import com.google.inject.name.Named;
+import org.apache.druid.tasklogs.TaskLogs;
+
+import javax.annotation.Nullable;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+
+public class SwitchingTaskLogs implements TaskLogs
+{
+
+ public static final String PROPERTY_PREFIX_SWITCHING =
"druid.indexer.logs.switching";
+ public static final String PROPERTY_KEY_SWITCHING_DEFAULT_TYPE =
PROPERTY_PREFIX_SWITCHING + ".defaultType";
Review Comment:
```suggestion
public static final String PROPERTY_DEFAULT_TYPE =
PROPERTY_PREFIX_SWITCHING + ".defaultType";
```
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/tasklogs/SwitchingTaskLogs.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.indexing.common.tasklogs;
+
+import com.google.common.base.Optional;
+import com.google.inject.Inject;
+import com.google.inject.name.Named;
+import org.apache.druid.tasklogs.TaskLogs;
+
+import javax.annotation.Nullable;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+
+public class SwitchingTaskLogs implements TaskLogs
+{
+
+ public static final String PROPERTY_PREFIX_SWITCHING =
"druid.indexer.logs.switching";
+ public static final String PROPERTY_KEY_SWITCHING_DEFAULT_TYPE =
PROPERTY_PREFIX_SWITCHING + ".defaultType";
+ public static final String PROPERTY_KEY_SWITCHING_PUSH_TYPE =
PROPERTY_PREFIX_SWITCHING + ".pushType";
+ public static final String PROPERTY_KEY_SWITCHING_STREAM_TYPE =
PROPERTY_PREFIX_SWITCHING + ".streamType";
+ public static final String PROPERTY_KEY_SWITCHING_REPORTS_TYPE =
PROPERTY_PREFIX_SWITCHING + ".reportsType";
+
+ public static final String KEY_SWITCHING_REPORTS = "switching.reportType";
+ public static final String KEY_SWITCHING_STREAM = "switching.streamType";
+ public static final String KEY_SWITCHING_PUSH = "switching.pushType";
+ public static final String KEY_SWITCHING_DEFAULT = "switching.defaultType";
+
+ private final TaskLogs reportTaskLogs;
+ private final TaskLogs logStreamer;
+ private final TaskLogs logPusher;
+
+ @Inject
+ public SwitchingTaskLogs(
+ @Nullable @Named(KEY_SWITCHING_DEFAULT) TaskLogs defaultTaskLogs,
+ @Nullable @Named(KEY_SWITCHING_REPORTS) TaskLogs reportTaskLogs,
+ @Nullable @Named(KEY_SWITCHING_STREAM) TaskLogs logStreamer,
+ @Nullable @Named(KEY_SWITCHING_PUSH) TaskLogs logPusher
+ )
+ {
+ this.reportTaskLogs = reportTaskLogs != null ? reportTaskLogs :
defaultTaskLogs;
Review Comment:
```suggestion
this.reportTaskLogs = Configs.valueOrDefault(reportTaskLogs,
defaultTaskLogs);
```
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/tasklogs/SwitchingTaskLogs.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.indexing.common.tasklogs;
+
+import com.google.common.base.Optional;
+import com.google.inject.Inject;
+import com.google.inject.name.Named;
+import org.apache.druid.tasklogs.TaskLogs;
+
+import javax.annotation.Nullable;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+
+public class SwitchingTaskLogs implements TaskLogs
+{
+
+ public static final String PROPERTY_PREFIX_SWITCHING =
"druid.indexer.logs.switching";
+ public static final String PROPERTY_KEY_SWITCHING_DEFAULT_TYPE =
PROPERTY_PREFIX_SWITCHING + ".defaultType";
+ public static final String PROPERTY_KEY_SWITCHING_PUSH_TYPE =
PROPERTY_PREFIX_SWITCHING + ".pushType";
+ public static final String PROPERTY_KEY_SWITCHING_STREAM_TYPE =
PROPERTY_PREFIX_SWITCHING + ".streamType";
+ public static final String PROPERTY_KEY_SWITCHING_REPORTS_TYPE =
PROPERTY_PREFIX_SWITCHING + ".reportsType";
+
+ public static final String KEY_SWITCHING_REPORTS = "switching.reportType";
+ public static final String KEY_SWITCHING_STREAM = "switching.streamType";
+ public static final String KEY_SWITCHING_PUSH = "switching.pushType";
+ public static final String KEY_SWITCHING_DEFAULT = "switching.defaultType";
+
+ private final TaskLogs reportTaskLogs;
+ private final TaskLogs logStreamer;
+ private final TaskLogs logPusher;
+
+ @Inject
+ public SwitchingTaskLogs(
+ @Nullable @Named(KEY_SWITCHING_DEFAULT) TaskLogs defaultTaskLogs,
+ @Nullable @Named(KEY_SWITCHING_REPORTS) TaskLogs reportTaskLogs,
Review Comment:
Should we call these `defaultDelegate` and `reportsDelegate` instead?
##########
indexing-service/src/main/java/org/apache/druid/indexing/common/tasklogs/SwitchingTaskLogs.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.indexing.common.tasklogs;
+
+import com.google.common.base.Optional;
+import com.google.inject.Inject;
+import com.google.inject.name.Named;
+import org.apache.druid.tasklogs.TaskLogs;
+
+import javax.annotation.Nullable;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+
+public class SwitchingTaskLogs implements TaskLogs
+{
+
+ public static final String PROPERTY_PREFIX_SWITCHING =
"druid.indexer.logs.switching";
+ public static final String PROPERTY_KEY_SWITCHING_DEFAULT_TYPE =
PROPERTY_PREFIX_SWITCHING + ".defaultType";
+ public static final String PROPERTY_KEY_SWITCHING_PUSH_TYPE =
PROPERTY_PREFIX_SWITCHING + ".pushType";
+ public static final String PROPERTY_KEY_SWITCHING_STREAM_TYPE =
PROPERTY_PREFIX_SWITCHING + ".streamType";
+ public static final String PROPERTY_KEY_SWITCHING_REPORTS_TYPE =
PROPERTY_PREFIX_SWITCHING + ".reportsType";
+
+ public static final String KEY_SWITCHING_REPORTS = "switching.reportType";
Review Comment:
```suggestion
public static final String NAME_REPORTS_TYPE = "switching.reportType";
```
##########
indexing-service/src/test/java/org/apache/druid/guice/IndexingServiceTaskLogsModuleTest.java:
##########
@@ -0,0 +1,142 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.guice;
+
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.google.inject.Key;
+import com.google.inject.Scopes;
+import com.google.inject.name.Names;
+import org.apache.druid.indexing.common.tasklogs.FileTaskLogs;
+import org.apache.druid.indexing.common.tasklogs.SwitchingTaskLogs;
+import org.apache.druid.jackson.JacksonModule;
+import org.apache.druid.tasklogs.NoopTaskLogs;
+import org.apache.druid.tasklogs.TaskLogs;
+import org.junit.Assert;
+import org.junit.Test;
+
+import javax.validation.ConstraintViolation;
+import javax.validation.Validator;
+import javax.validation.executable.ExecutableValidator;
+import javax.validation.metadata.BeanDescriptor;
+import java.util.Collections;
+import java.util.Properties;
+import java.util.Set;
+
+public class IndexingServiceTaskLogsModuleTest
+{
+
+ @Test
+ public void test_switchingTaskLogs_shouldUseDefaultWhenNoPusherConfigured()
+ {
+ Properties props = new Properties();
+ props.setProperty("druid.indexer.logs.type", "switching");
+ props.setProperty("druid.indexer.logs.switching.defaultType", "file");
+
+ Injector injector = Guice.createInjector(
+ binder -> {
+ binder.bindScope(LazySingleton.class, Scopes.SINGLETON);
+ binder.bind(Properties.class).toInstance(props);
+ binder.bind(Validator.class).toInstance(new MockValidator());
+ },
+ new IndexingServiceTaskLogsModule(props),
+ new JacksonModule()
+ );
+
+ TaskLogs taskLogs = injector.getInstance(TaskLogs.class);
+ Assert.assertTrue(taskLogs instanceof SwitchingTaskLogs);
+ }
+
+ @Test
+ public void test_providePusher_shouldReturnConfiguredImplementaions()
+ {
+ Properties props = new Properties();
+ props.setProperty("druid.indexer.logs.type", "switching");
+ props.setProperty("druid.indexer.logs.switching.defaultType", "noop");
+ props.setProperty("druid.indexer.logs.switching.pushType", "file");
+ props.setProperty("druid.indexer.logs.switching.reportsType", "noop");
+ props.setProperty("druid.indexer.logs.switching.streamType", "file");
+
+ Injector injector = Guice.createInjector(
+ binder -> {
+ binder.bindScope(LazySingleton.class, Scopes.SINGLETON);
+ binder.bind(Properties.class).toInstance(props);
+ binder.bind(Validator.class).toInstance(new MockValidator());
+ },
+ new IndexingServiceTaskLogsModule(props),
+ new JacksonModule()
+ );
+
+ TaskLogs pusher = injector.getInstance(Key.get(TaskLogs.class,
Names.named("switching.pushType")));
+ Assert.assertTrue(pusher instanceof FileTaskLogs);
+ TaskLogs reports = injector.getInstance(Key.get(TaskLogs.class,
Names.named("switching.reportType")));
Review Comment:
Nit: Please use consistent names, e.g. `logPushType`, `reportsType`,
`logStreamType`.
##########
indexing-service/src/test/java/org/apache/druid/indexing/common/tasklogs/SwitchingTaskLogsTest.java:
##########
@@ -0,0 +1,125 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.indexing.common.tasklogs;
+
+import com.google.common.base.Optional;
+import org.apache.druid.tasklogs.TaskLogs;
+import org.easymock.EasyMock;
+import org.easymock.EasyMockRunner;
+import org.easymock.EasyMockSupport;
+import org.easymock.Mock;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import java.io.ByteArrayInputStream;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
+
+@RunWith(EasyMockRunner.class)
+public class SwitchingTaskLogsTest extends EasyMockSupport
+{
+ @Mock
+ private TaskLogs defaultTaskLogs;
+
+ @Mock
+ private TaskLogs reportTaskLogs;
+
+ @Mock
+ private TaskLogs streamerTaskLogs;
+
+ @Mock
+ private TaskLogs pusherTaskLogs;
+
+ private SwitchingTaskLogs taskLogs;
+
+ @Before
+ public void setUp()
+ {
+ taskLogs = new SwitchingTaskLogs(defaultTaskLogs, reportTaskLogs,
streamerTaskLogs, pusherTaskLogs);
+ }
+
+ @Test
+ public void test_shouldUseIndividualLogs() throws IOException
Review Comment:
It would be nice to break up this test and perform individual verification
for each of the methods.
##########
indexing-service/src/test/java/org/apache/druid/guice/IndexingServiceTaskLogsModuleTest.java:
##########
@@ -0,0 +1,142 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.guice;
+
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.google.inject.Key;
+import com.google.inject.Scopes;
+import com.google.inject.name.Names;
+import org.apache.druid.indexing.common.tasklogs.FileTaskLogs;
+import org.apache.druid.indexing.common.tasklogs.SwitchingTaskLogs;
+import org.apache.druid.jackson.JacksonModule;
+import org.apache.druid.tasklogs.NoopTaskLogs;
+import org.apache.druid.tasklogs.TaskLogs;
+import org.junit.Assert;
+import org.junit.Test;
+
+import javax.validation.ConstraintViolation;
+import javax.validation.Validator;
+import javax.validation.executable.ExecutableValidator;
+import javax.validation.metadata.BeanDescriptor;
+import java.util.Collections;
+import java.util.Properties;
+import java.util.Set;
+
+public class IndexingServiceTaskLogsModuleTest
+{
+
+ @Test
+ public void test_switchingTaskLogs_shouldUseDefaultWhenNoPusherConfigured()
Review Comment:
```suggestion
public void test_switchingTaskLogs_usesDefaultType_whenNoPusherConfigured()
```
--
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]
