[GitHub] zookeeper pull request #148: ZOOKEEPER-2659 Log4j 2 migration
GitHub user praste reopened a pull request: https://github.com/apache/zookeeper/pull/148 ZOOKEEPER-2659 Log4j 2 migration You can merge this pull request into a Git repository by running: $ git pull https://github.com/praste/zookeeper log4j2-migration Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/148.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #148 commit 6141943989c65519a8ddcb346aeda79f9cab81ae Author: Pushkar Raste Date: 2016-12-20T22:47:24Z Log4j2 migration commit f31eaa95e5ef3a884737dd7c1e56da8dea48c58c Author: Pushkar Raste Date: 2016-12-20T22:48:17Z Log4j2 migration commit 5770af750e90e09ab81a362ddcfbc6d25b6db2a5 Author: Pushkar Raste Date: 2017-01-04T19:57:11Z Log4j migration commit 87f307592ca96c00e7c536f661715f8a6051de8d Author: Pushkar Raste Date: 2017-01-04T22:32:16Z Log4j2 migration commit fe072cee365c09e1fb1c5bb8df754c0da78cd471 Author: Pushkar Raste Date: 2017-01-05T21:00:23Z Log4j migration commit 995277f5e80c998a0fa49fbd6b74ea53d7bf3468 Author: Pushkar Raste Date: 2017-01-13T16:35:49Z Log4j-migration Added disruptor dependency commit 1d94d27f3caf29e9e582f9778ab79e10abaec35e Author: Pushkar Raste Date: 2017-01-13T16:43:33Z Merge branch 'master' of https://github.com/apache/zookeeper into log4j2-migration commit abc51db3ad3dbd30838fd72b782f8d41c7c808a0 Author: Pushkar Raste Date: 2017-01-13T18:35:43Z Log4j2 migration commit 42a587d8862892a27bbb9fbbe7ba1ca4dcb0f42f Author: Pushkar Raste Date: 2017-01-14T02:13:21Z Log4j2 migration commit 2db983bc3e449c4220eddd4f720f5e4233ea3d55 Author: Pushkar Raste Date: 2017-01-20T14:02:17Z Changes as per review comments commit 77214d1d4b45d572633450ffb54310d6fd17c8dc Author: Pushkar Raste Date: 2017-01-21T17:23:06Z Fix ivy pattern --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #148: ZOOKEEPER-2659 Log4j 2 migration
Github user praste commented on the issue: https://github.com/apache/zookeeper/pull/148 Reopening as multiple people are interested in it. However, I have switched jobs and may not be able to make further changes until I get approval from my new employer. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #148: ZOOKEEPER-2659 Log4j 2 migration
Github user praste closed the pull request at: https://github.com/apache/zookeeper/pull/148 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #148: ZOOKEEPER-2659 Log4j 2 migration
Github user praste commented on the issue: https://github.com/apache/zookeeper/pull/148 Looks like no one is really interested in this. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #148: ZOOKEEPER-2659 Log4j 2 migration
Github user praste commented on the issue: https://github.com/apache/zookeeper/pull/148 can this be merged ? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #148: ZOOKEEPER-2659 Log4j 2 migration
Github user praste commented on the issue: https://github.com/apache/zookeeper/pull/148 Two options 1. Use patch * Use link https://patch-diff.githubusercontent.com/raw/apache/zookeeper/pull/148.diff and save it as patch. Then use `git apply-patch` 2. Pull from my fork (I have not really tested following steps, jut relying on my muscle memory) * `git remote add praste https://github.com/praste/zookeeper/` * `git fetch praste` * `git pull praste/log4j2-migration` --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #148: ZOOKEEPER-2659 Log4j 2 migration
Github user praste commented on the issue: https://github.com/apache/zookeeper/pull/148 Are sure you removed reference to `slf4j-log4j12` from all the `ivy.xml` files? I am not an ivy expert but you can take a look at http://stackoverflow.com/questions/5405310/find-hidden-dependencies-in-ivy and http://ant.apache.org/ivy/history/latest-milestone/use/dependencytree.html --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #148: ZOOKEEPER-2659 Log4j 2 migration
Github user praste commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/148#discussion_r98013864 --- Diff: src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java --- @@ -413,13 +418,18 @@ public void testBadPeerAddressInQuorum() throws Exception { ClientBase.setupTestEnv(); // setup the logger to capture all logs +LoggerContext loggerContext = (LoggerContext) LogManager.getContext(false); --- End diff -- @jvz is using ListAppender absolutely necessary? Can we merge this with current changes? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #148: ZOOKEEPER-2659 Log4j 2 migration
Github user praste commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/148#discussion_r97232914 --- Diff: src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java --- @@ -413,13 +418,18 @@ public void testBadPeerAddressInQuorum() throws Exception { ClientBase.setupTestEnv(); // setup the logger to capture all logs +LoggerContext loggerContext = (LoggerContext) LogManager.getContext(false); --- End diff -- @jvz - I tried to update tests using the `ListAppender` but I don't think, it is the right way to verify the log messages. I observed that when I run all the tests in the `QuorumPeerMainTest` or `ReadOnlyModeTest`, tests fail. However, running following test using `test.method=` option works fine `QuorumPeerMainTest. testInconsistentPeerType` `QuorumPeerMainTest. testQuorumDefaults` `QuorumPeerMainTest. testBadPeerAddressInQuorum` `ReadOnlyModeTest.testSeekForRwServer` Not sure what am I missing here. Let me know what you think. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #148: ZOOKEEPER-2659 Log4j 2 migration
Github user praste commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/148#discussion_r96629041 --- Diff: src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java --- @@ -413,13 +418,18 @@ public void testBadPeerAddressInQuorum() throws Exception { ClientBase.setupTestEnv(); // setup the logger to capture all logs +LoggerContext loggerContext = (LoggerContext) LogManager.getContext(false); --- End diff -- I am not familiar with Ivy. any idea how to add maven dependencies with tests qualifier in Ivy? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper issue #148: ZOOKEEPER-2659 Log4j 2 migration
Github user praste commented on the issue: https://github.com/apache/zookeeper/pull/148 I have made all the suggested changes except for using List appender, which I will look into tonight --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #148: ZOOKEEPER-2659 Log4j 2 migration
Github user praste commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/148#discussion_r96327023 --- Diff: src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java --- @@ -413,13 +418,18 @@ public void testBadPeerAddressInQuorum() throws Exception { ClientBase.setupTestEnv(); // setup the logger to capture all logs +LoggerContext loggerContext = (LoggerContext) LogManager.getContext(false); --- End diff -- @jvz Thanks for the suggestion, will look into it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #148: ZOOKEEPER-2659 Log4j 2 migration
Github user praste commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/148#discussion_r96277825 --- Diff: conf/log4j2.xml --- @@ -0,0 +1,56 @@ + + + + +%d{ISO8601} [myid:%X{myid}] - %-5p [%t:%C{1}@%L] - %m%n --- End diff -- I was trying to make sure that I can generate log4j2.xml configuration compatible with the current log4j.properties --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #148: ZOOKEEPER-2659 Log4j 2 migration
Github user praste commented on a diff in the pull request: https://github.com/apache/zookeeper/pull/148#discussion_r96277523 --- Diff: ivy.xml --- @@ -41,13 +41,20 @@ - + + + + + + + + - + --- End diff -- I don't think we should be using 'log4j 1' at all --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zookeeper pull request #148: ZOOKEEPER-2664 Log4j2 migration
GitHub user praste opened a pull request: https://github.com/apache/zookeeper/pull/148 ZOOKEEPER-2664 Log4j2 migration You can merge this pull request into a Git repository by running: $ git pull https://github.com/praste/zookeeper log4j2-migration Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zookeeper/pull/148.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #148 commit 6141943989c65519a8ddcb346aeda79f9cab81ae Author: Pushkar Raste Date: 2016-12-20T22:47:24Z Log4j2 migration commit f31eaa95e5ef3a884737dd7c1e56da8dea48c58c Author: Pushkar Raste Date: 2016-12-20T22:48:17Z Log4j2 migration commit 5770af750e90e09ab81a362ddcfbc6d25b6db2a5 Author: Pushkar Raste Date: 2017-01-04T19:57:11Z Log4j migration commit 87f307592ca96c00e7c536f661715f8a6051de8d Author: Pushkar Raste Date: 2017-01-04T22:32:16Z Log4j2 migration commit fe072cee365c09e1fb1c5bb8df754c0da78cd471 Author: Pushkar Raste Date: 2017-01-05T21:00:23Z Log4j migration commit 995277f5e80c998a0fa49fbd6b74ea53d7bf3468 Author: Pushkar Raste Date: 2017-01-13T16:35:49Z Log4j-migration Added disruptor dependency commit 1d94d27f3caf29e9e582f9778ab79e10abaec35e Author: Pushkar Raste Date: 2017-01-13T16:43:33Z Merge branch 'master' of https://github.com/apache/zookeeper into log4j2-migration --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---