SAMZA-2155: Remove log4j log4j2 dependency from samza-test #985
Conversation
|
@prateekm for review! |
| compile project(":samza-core_$scalaSuffix") | ||
| compile project(":samza-kafka_$scalaSuffix") | ||
| compile project(":samza-sql_$scalaSuffix") | ||
| runtime project(":samza-log4j_$scalaSuffix") |
There was a problem hiding this comment.
Do you need to exclude the slf4j-log4j12 (log4j1 binding) and slf4j-log4j-impl (log4j2 binding) from the dependencies? At least log4j1 binding currently comes transitively from several of the modules. If SLF4J picks it up and it finds that the log4j.xml is missing, it might not log to console.
There was a problem hiding this comment.
+1, it'll log to stdout, but thats still not desirable
There was a problem hiding this comment.
@rmatharu Actually logging to stdout is what we want, unless you think it'll be better to have log files for tests. For that, we already have a testRuntime dependency on slf4j-simple (console logger). My concern is that it will pick log4j1 binding, find that the xml file is missing, and not log anything (like you saw in the diagnostics repartitioner)
There was a problem hiding this comment.
@prateekm yup that safeguard is good to have.
gradle :samza-test:test -i with that safeguard exclude logs to console
| # Job | ||
| job.factory.class=samza.job.local.ThreadJobFactory | ||
| job.name=hello-stateful-world | ||
| app.name=hello-stateful-world |
There was a problem hiding this comment.
Perhaps you need to update shouldStopNewProcessorsJoiningGroupWhenNumContainersIsGreaterThanNumTasks
which is failing due to this change.
There was a problem hiding this comment.
Failing test is not related to this change
3 participants
No description provided.