You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2020/07/25 13:08:01 UTC

[GitHub] [pulsar] lbenc135 opened a new pull request #7662: Enable Log4cxx support

lbenc135 opened a new pull request #7662:
URL: https://github.com/apache/pulsar/pull/7662


   <!--
   ### Contribution Checklist
     
     - Name the pull request in the form "[Issue XYZ][component] Title of the pull request", where *XYZ* should be replaced by the actual issue number.
       Skip *Issue XYZ* if there is no associated github issue for this pull request.
       Skip *component* if you are unsure about which is the best component. E.g. `[docs] Fix typo in produce method`.
   
     - Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
     
     - Each pull request should address only one issue, not mix up code from multiple issues.
     
     - Each commit in the pull request has a meaningful commit message
   
     - Once all items of the checklist are addressed, remove the above text and this checklist, leaving only the filled out template below.
   
   **(The sections below can be removed for hotfixes of typos)**
   -->
   
   Partially solves #4234
   
   ### Motivation
   
   The C++ and Python client's logging is not configurable and the stdout is spammed with pulsar logs.
   
   The clients provides an option of specifying a `log4cxx.conf` file, but this had no effect because it was ignored.
   
   
   ### Modifications
   
   Enabled Log4cxx and fixed the Log4Cxx logger (some leftovers from previous refactor).
   
   ### Verifying this change
   
   - [ ] Make sure that the change passes the CI checks.
   
   Not sure how to test this with unit tests, but I compiled it locally, tried it out, even used it in a project, and it works.
   
   ### Does this pull request potentially affect one of the following parts:
   
   *If `yes` was chosen, please highlight the changes*
   
     - Dependencies (does it add or upgrade a dependency): yes (maybe)
     - The public API: no
     - The schema: no
     - The default values of configurations: no
     - The wire protocol: no
     - The rest endpoints: no
     - The admin cli options: no
     - Anything that affects deployment: don't know
   
   ### Documentation
   
     - Does this pull request introduce a new feature? no
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [pulsar] Giaco9 commented on pull request #7662: Fix Log4cxx support

Posted by GitBox <gi...@apache.org>.
Giaco9 commented on pull request #7662:
URL: https://github.com/apache/pulsar/pull/7662#issuecomment-851857255


   Hi everyone, any news on this issue? We would like to configure the python logger in version prior to 2.8. Would this pull request make it possible? Also, this pull request https://github.com/apache/pulsar/issues/7838 seems to be helpful, which one will be merged? Is any help needed?
   
   Thank you
   
   Giacomo


-- 
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:
users@infra.apache.org



[GitHub] [pulsar] lbenc135 commented on pull request #7662: Fix Log4cxx support

Posted by GitBox <gi...@apache.org>.
lbenc135 commented on pull request #7662:
URL: https://github.com/apache/pulsar/pull/7662#issuecomment-667298988


   @merlimat I revived the change here https://github.com/apache/pulsar/pull/7713 so please take a look if you can.
   
   I reverted the CMake flag here and left only the refactoring fixes. Not sure why the CI failed though.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [pulsar] lbenc135 commented on pull request #7662: Enable Log4cxx support

Posted by GitBox <gi...@apache.org>.
lbenc135 commented on pull request #7662:
URL: https://github.com/apache/pulsar/pull/7662#issuecomment-663935658


   @merlimat I see, thanks for the explanation. Does that mean that I can't use the default distribution (from PyPI for Python) and I have to build my own from source?
   
   Should I maybe investigate other approaches to control logging? Maybe we can do something similar to what the Kafka client does - create a logger PyObject (to fix #4234). Or we can add a method/argument on the client for controlling the log level.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [pulsar] merlimat commented on pull request #7662: Enable Log4cxx support

Posted by GitBox <gi...@apache.org>.
merlimat commented on pull request #7662:
URL: https://github.com/apache/pulsar/pull/7662#issuecomment-663862291


   @lbenc135 the CMake flag was meant so that you can decide to use Log4Cxx. It's not a good idea to have it enabled by default, since on many platforms Log4Cxx can be very complicated to install (because of its dependencies).


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [pulsar] merlimat commented on pull request #7662: Enable Log4cxx support

Posted by GitBox <gi...@apache.org>.
merlimat commented on pull request #7662:
URL: https://github.com/apache/pulsar/pull/7662#issuecomment-663862400


   just do `cmake . -DUSE_LOG4CXX=ON` 


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [pulsar] github-actions[bot] commented on pull request #7662: Fix Log4cxx support

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #7662:
URL: https://github.com/apache/pulsar/pull/7662#issuecomment-1058899590


   @lbenc135:Thanks for your contribution. For this PR, do we need to update docs?
   (The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)


-- 
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: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org