bkonold commented on a change in pull request #1403:
URL: https://github.com/apache/samza/pull/1403#discussion_r462642571
##########
File path:
samza-log4j2/src/main/java/org/apache/samza/logging/log4j2/StreamAppender.java
##########
@@ -67,48 +69,52 @@
import org.apache.samza.system.SystemStream;
import org.apache.samza.util.ExponentialSleepStrategy;
import org.apache.samza.util.HttpUtil;
+import org.apache.samza.util.MetricsReporterLoader;
import org.apache.samza.util.ReflectionUtil;
@Plugin(name = "Stream", category = "Core", elementType = "appender",
printObject = true)
public class StreamAppender extends AbstractAppender {
- private static final String JAVA_OPTS_CONTAINER_NAME =
"samza.container.name";
- private static final String JOB_COORDINATOR_TAG = "samza-job-coordinator";
- private static final String SOURCE = "log4j-log";
+ protected static final String JAVA_OPTS_CONTAINER_NAME =
"samza.container.name";
+ protected static final String JOB_COORDINATOR_TAG = "samza-job-coordinator";
+ protected static final String SOURCE = "log4j-log";
Review comment:
Agree I've seen partial protection in several places of Samza so I think
it would be fine here. Labeling only what you need to as protected is also
better for the reader because it will restrict the set of functionality a
subclass can override. IMO, I'd be in favor of labeling only what you need to
as protected.
----------------------------------------------------------------
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]
