You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@curator.apache.org by karel1980 <gi...@git.apache.org> on 2014/08/14 14:15:14 UTC
[GitHub] curator pull request: CURATOR-141 Don't make calls to the client a...
GitHub user karel1980 opened a pull request:
https://github.com/apache/curator/pull/37
CURATOR-141 Don't make calls to the client after PathChildrenCache has been closed
The test case is a bit weird because it needed to intercept logging.
Also, I've added slf4j-log4j12 as a test dependency to control logging.
I think it should be added to the other poms as well, otherwise logging
seems to get lost.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/karel1980/curator CURATOR-141
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/curator/pull/37.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 #37
----
commit f2f1953c544398cb6db53a2ac59b02719cc83fa3
Author: Karel Vervaeke <ka...@ngdata.com>
Date: 2014-08-14T11:46:03Z
CURATOR-141 Don't make calls to the client after PathChildrenCache has been closed.
The test case is a bit weird because it needed to intercept logging.
Also, I've added slf4j-log4j12 as a test dependency to control logging.
I think it should be added to the other poms as well, otherwise logging
seems to get lost.
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] curator pull request: CURATOR-141 Don't make calls to the client a...
Posted by karel1980 <gi...@git.apache.org>.
Github user karel1980 commented on the pull request:
https://github.com/apache/curator/pull/37#issuecomment-52482025
It should be pointed out that the scope of the slf4j-log4j12 dependency is 'test' so it only affects the tests.
Without this, no slf4j binding is present during the test which causes logging to be incompletely set up.
You can see this by the messages in target/surefire-reports/*-output.txt:
SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.
Unfortunately revisiting this I noticed that the 'Just checking' message for verifying the slf4j setup fails to break the test when the slf4j binding is removed from the pom. Not what I wanted. I'm having a closer look to see what goes wrong and how it can be improved.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] curator pull request: CURATOR-141 Don't make calls to the client a...
Posted by madrob <gi...@git.apache.org>.
Github user madrob commented on the pull request:
https://github.com/apache/curator/pull/37#issuecomment-52509064
I have #36 open explicitly for the test bindings.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] curator pull request: CURATOR-141 Don't make calls to the client a...
Posted by cammckenzie <gi...@git.apache.org>.
Github user cammckenzie commented on the pull request:
https://github.com/apache/curator/pull/37#issuecomment-52276174
Fix looks good to me, as does the test. I won't merge the request quite yet though as I'm unsure of the POM changes. While they look ok to me, I'm a complete Maven newbie and I'm not sure if these dependencies should be pulled in in another way. Presumably the build is picking up the slf4j libraries already given that the code uses them?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] curator pull request: CURATOR-141 Don't make calls to the client a...
Posted by karel1980 <gi...@git.apache.org>.
Github user karel1980 commented on the pull request:
https://github.com/apache/curator/pull/37#issuecomment-52485439
I've changed the way we detect the correct log setup for our test.
I hope together with my previous comments things are sufficiently cleared up now.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] curator pull request: CURATOR-141 Don't make calls to the client a...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/curator/pull/37
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---