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/31 18:24:18 UTC
[GitHub] [pulsar] lbenc135 opened a new pull request #7713: implement python logger wrapper
lbenc135 opened a new pull request #7713:
URL: https://github.com/apache/pulsar/pull/7713
<!--
### 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)**
-->
Fixes #4234
### Motivation
Logging in the python client is not configurable, so the stdout was spammed. This PR revives the https://github.com/apache/pulsar/pull/6113 but tries to resolve the bugs that resulted the last time.
### Modifications
Implemented a python logger wrapper. You can pass a python logger as an argument to the client.
### Verifying this change
- [ ] Make sure that the change passes the CI checks.
Not sure how to unit-test this. I tested it manually by building the client and trying it out with python scripts. It works as expected both with the logger forwarded and without. Also, configuring the logger also configures the log level so this fixes the problem.
### 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): no
- The public API: yes (?)
- 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: no
### Documentation
- Does this pull request introduce a new feature? yes
- If yes, how is the feature documented? inline function docs
----------------------------------------------------------------
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] Bklyn commented on a change in pull request #7713: implement python logger wrapper
Posted by GitBox <gi...@apache.org>.
Bklyn commented on a change in pull request #7713:
URL: https://github.com/apache/pulsar/pull/7713#discussion_r598825194
##########
File path: pulsar-client-cpp/python/src/config.cc
##########
@@ -88,6 +88,107 @@ static ProducerConfiguration& ProducerConfiguration_setCryptoKeyReader(ProducerC
return conf;
}
+class LoggerWrapper: public Logger {
+ std::string _logger;
+ PyObject* _pyLogger;
+ int _currentPythonLogLevel = _getLogLevelValue(Logger::LEVEL_INFO);
+
+ void _updateCurrentPythonLogLevel() {
+ PyGILState_STATE state = PyGILState_Ensure();
+
+ try {
+ _currentPythonLogLevel = py::call_method<int>(_pyLogger, "getEffectiveLevel");
+ } catch (py::error_already_set e) {
+ PyErr_Print();
+ }
+
+ PyGILState_Release(state);
+ };
+
+ int _getLogLevelValue(Level level) {
+ return 10 + (level * 10);
+ }
+
+ public:
+
+ LoggerWrapper(const std::string &logger, PyObject* pyLogger) : _logger(logger) {
+ _pyLogger = pyLogger;
+ Py_XINCREF(_pyLogger);
+
+ _updateCurrentPythonLogLevel();
+ }
+
+ LoggerWrapper(const LoggerWrapper& other) {
+ _pyLogger = other._pyLogger;
+ Py_XINCREF(_pyLogger);
+ }
+
+ LoggerWrapper& operator=(const LoggerWrapper& other) {
Review comment:
As above, but is the `_logger` member even used? Maybe you should drop it.
--
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] pinkavaj commented on a change in pull request #7713: implement python logger wrapper
Posted by GitBox <gi...@apache.org>.
pinkavaj commented on a change in pull request #7713:
URL: https://github.com/apache/pulsar/pull/7713#discussion_r562161863
##########
File path: pulsar-client-cpp/python/src/config.cc
##########
@@ -74,6 +74,107 @@ static ClientConfiguration& ClientConfiguration_setAuthentication(ClientConfigur
return conf;
}
+class LoggerWrapper: public Logger {
+ std::string _logger;
+ PyObject* _pyLogger;
+ int _currentPythonLogLevel = _getLogLevelValue(Logger::LEVEL_INFO);
+
+ void _updateCurrentPythonLogLevel() {
+ PyGILState_STATE state = PyGILState_Ensure();
+
+ try {
+ _currentPythonLogLevel = py::call_method<int>(_pyLogger, "getEffectiveLevel");
+ } catch (py::error_already_set e) {
+ PyErr_Print();
+ }
+
+ PyGILState_Release(state);
+ };
+
+ int _getLogLevelValue(Level level) {
+ return 10 + (level * 10);
+ }
+
+ public:
+
+ LoggerWrapper(const std::string &logger, PyObject* pyLogger) : _logger(logger) {
+ _pyLogger = pyLogger;
+ Py_XINCREF(_pyLogger);
+
+ _updateCurrentPythonLogLevel();
+ }
+
+ LoggerWrapper(const LoggerWrapper& other) {
+ _pyLogger = other._pyLogger;
+ Py_XINCREF(_pyLogger);
+ }
+
+ LoggerWrapper& operator=(const LoggerWrapper& other) {
+ _pyLogger = other._pyLogger;
+ Py_XINCREF(_pyLogger);
+ return *this;
+ }
+
+ virtual ~LoggerWrapper() {
+ Py_XDECREF(_pyLogger);
+ }
+
+ bool isEnabled(Level level) {
+ return _getLogLevelValue(level) >= _currentPythonLogLevel;
+ }
+
+ void log(Level level, int line, const std::string& message) {
Review comment:
The file and line information is not logged, but I'm not familiar with the code enought to decide what is more appropriate. From the point of developer I would preffer the file and line information in the log message.
Probably someone more familiar with this code should speek his opinion.
Otherwise looks good.
----------------------------------------------------------------
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 #7713: implement python logger wrapper
Posted by GitBox <gi...@apache.org>.
Giaco9 commented on pull request #7713:
URL: https://github.com/apache/pulsar/pull/7713#issuecomment-844885547
Hi everyone, thank you for the effort put in this pull request! Would be possible to release a version compatible with python2.7 that includes these changes?
Thank you!
--
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 a change in pull request #7713: implement python logger wrapper
Posted by GitBox <gi...@apache.org>.
lbenc135 commented on a change in pull request #7713:
URL: https://github.com/apache/pulsar/pull/7713#discussion_r598090448
##########
File path: pulsar-client-cpp/python/src/config.cc
##########
@@ -74,6 +74,107 @@ static ClientConfiguration& ClientConfiguration_setAuthentication(ClientConfigur
return conf;
}
+class LoggerWrapper: public Logger {
+ std::string _logger;
+ PyObject* _pyLogger;
+ int _currentPythonLogLevel = _getLogLevelValue(Logger::LEVEL_INFO);
+
+ void _updateCurrentPythonLogLevel() {
+ PyGILState_STATE state = PyGILState_Ensure();
+
+ try {
+ _currentPythonLogLevel = py::call_method<int>(_pyLogger, "getEffectiveLevel");
+ } catch (py::error_already_set e) {
+ PyErr_Print();
+ }
+
+ PyGILState_Release(state);
+ };
+
+ int _getLogLevelValue(Level level) {
+ return 10 + (level * 10);
+ }
+
+ public:
+
+ LoggerWrapper(const std::string &logger, PyObject* pyLogger) : _logger(logger) {
+ _pyLogger = pyLogger;
+ Py_XINCREF(_pyLogger);
+
+ _updateCurrentPythonLogLevel();
+ }
+
+ LoggerWrapper(const LoggerWrapper& other) {
+ _pyLogger = other._pyLogger;
+ Py_XINCREF(_pyLogger);
+ }
+
+ LoggerWrapper& operator=(const LoggerWrapper& other) {
+ _pyLogger = other._pyLogger;
+ Py_XINCREF(_pyLogger);
+ return *this;
+ }
+
+ virtual ~LoggerWrapper() {
+ Py_XDECREF(_pyLogger);
+ }
+
+ bool isEnabled(Level level) {
+ return _getLogLevelValue(level) >= _currentPythonLogLevel;
+ }
+
+ void log(Level level, int line, const std::string& message) {
Review comment:
Thank you @Bklyn, please consider approving the PR
--
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 #7713: implement python logger wrapper
Posted by GitBox <gi...@apache.org>.
lbenc135 commented on pull request #7713:
URL: https://github.com/apache/pulsar/pull/7713#issuecomment-751631553
@pinkavaj could you take another look please?
----------------------------------------------------------------
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] pinkavaj commented on a change in pull request #7713: implement python logger wrapper
Posted by GitBox <gi...@apache.org>.
pinkavaj commented on a change in pull request #7713:
URL: https://github.com/apache/pulsar/pull/7713#discussion_r562161863
##########
File path: pulsar-client-cpp/python/src/config.cc
##########
@@ -74,6 +74,107 @@ static ClientConfiguration& ClientConfiguration_setAuthentication(ClientConfigur
return conf;
}
+class LoggerWrapper: public Logger {
+ std::string _logger;
+ PyObject* _pyLogger;
+ int _currentPythonLogLevel = _getLogLevelValue(Logger::LEVEL_INFO);
+
+ void _updateCurrentPythonLogLevel() {
+ PyGILState_STATE state = PyGILState_Ensure();
+
+ try {
+ _currentPythonLogLevel = py::call_method<int>(_pyLogger, "getEffectiveLevel");
+ } catch (py::error_already_set e) {
+ PyErr_Print();
+ }
+
+ PyGILState_Release(state);
+ };
+
+ int _getLogLevelValue(Level level) {
+ return 10 + (level * 10);
+ }
+
+ public:
+
+ LoggerWrapper(const std::string &logger, PyObject* pyLogger) : _logger(logger) {
+ _pyLogger = pyLogger;
+ Py_XINCREF(_pyLogger);
+
+ _updateCurrentPythonLogLevel();
+ }
+
+ LoggerWrapper(const LoggerWrapper& other) {
+ _pyLogger = other._pyLogger;
+ Py_XINCREF(_pyLogger);
+ }
+
+ LoggerWrapper& operator=(const LoggerWrapper& other) {
+ _pyLogger = other._pyLogger;
+ Py_XINCREF(_pyLogger);
+ return *this;
+ }
+
+ virtual ~LoggerWrapper() {
+ Py_XDECREF(_pyLogger);
+ }
+
+ bool isEnabled(Level level) {
+ return _getLogLevelValue(level) >= _currentPythonLogLevel;
+ }
+
+ void log(Level level, int line, const std::string& message) {
Review comment:
The file and line information is not logged, but I'm not familiar with the code enought to decide what is more appropriate. From the point of developer I would preffer the file and line information in the log message.
Probably someone more familiar with this code should speek his opinion.
Otherwise looks good.
##########
File path: pulsar-client-cpp/python/src/config.cc
##########
@@ -74,6 +74,107 @@ static ClientConfiguration& ClientConfiguration_setAuthentication(ClientConfigur
return conf;
}
+class LoggerWrapper: public Logger {
+ std::string _logger;
+ PyObject* _pyLogger;
+ int _currentPythonLogLevel = _getLogLevelValue(Logger::LEVEL_INFO);
+
+ void _updateCurrentPythonLogLevel() {
+ PyGILState_STATE state = PyGILState_Ensure();
+
+ try {
+ _currentPythonLogLevel = py::call_method<int>(_pyLogger, "getEffectiveLevel");
+ } catch (py::error_already_set e) {
+ PyErr_Print();
+ }
+
+ PyGILState_Release(state);
+ };
+
+ int _getLogLevelValue(Level level) {
+ return 10 + (level * 10);
+ }
+
+ public:
+
+ LoggerWrapper(const std::string &logger, PyObject* pyLogger) : _logger(logger) {
+ _pyLogger = pyLogger;
+ Py_XINCREF(_pyLogger);
+
+ _updateCurrentPythonLogLevel();
+ }
+
+ LoggerWrapper(const LoggerWrapper& other) {
+ _pyLogger = other._pyLogger;
+ Py_XINCREF(_pyLogger);
+ }
+
+ LoggerWrapper& operator=(const LoggerWrapper& other) {
+ _pyLogger = other._pyLogger;
+ Py_XINCREF(_pyLogger);
+ return *this;
+ }
+
+ virtual ~LoggerWrapper() {
+ Py_XDECREF(_pyLogger);
+ }
+
+ bool isEnabled(Level level) {
+ return _getLogLevelValue(level) >= _currentPythonLogLevel;
+ }
+
+ void log(Level level, int line, const std::string& message) {
Review comment:
Feel free to show example and you incentive to help with decission.
----------------------------------------------------------------
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] tuteng commented on pull request #7713: implement python logger wrapper
Posted by GitBox <gi...@apache.org>.
tuteng commented on pull request #7713:
URL: https://github.com/apache/pulsar/pull/7713#issuecomment-808634161
/pulsarbot run-failure-checks
--
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] tuteng commented on pull request #7713: implement python logger wrapper
Posted by GitBox <gi...@apache.org>.
tuteng commented on pull request #7713:
URL: https://github.com/apache/pulsar/pull/7713#issuecomment-807726201
/pulsarbot run-failure-checks
--
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] mianos commented on pull request #7713: implement python logger wrapper
Posted by GitBox <gi...@apache.org>.
mianos commented on pull request #7713:
URL: https://github.com/apache/pulsar/pull/7713#issuecomment-871025689
Now this is merged I can't seem to get it working. I suspect it is because this logger is not loaded in the C++.
I would expect the massive number of INFO messages would be suppressed by doing the following:
```
plogger = logging.getLogger("pulsar")
print(f"current level {plogger.getEffectiveLevel()}")
plogger.info("Hello info")
plogger.setLevel(logging.ERROR)
print(f"post setting current level {plogger.getEffectiveLevel()}")
plogger.info("post unset info Hello info")
client = pulsar.Client(pulsar_config.uri, logger=plogger)
```
I see the normal python logger suppresses the info messages but the prefect-client 2.8 still prints all
the INFO messages.
Maybe the unit test should set a logger handler and check it's not full of INFO messages?
--
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
[GitHub] [pulsar] Bklyn commented on a change in pull request #7713: implement python logger wrapper
Posted by GitBox <gi...@apache.org>.
Bklyn commented on a change in pull request #7713:
URL: https://github.com/apache/pulsar/pull/7713#discussion_r598823629
##########
File path: pulsar-client-cpp/python/src/config.cc
##########
@@ -88,6 +88,107 @@ static ProducerConfiguration& ProducerConfiguration_setCryptoKeyReader(ProducerC
return conf;
}
+class LoggerWrapper: public Logger {
+ std::string _logger;
+ PyObject* _pyLogger;
+ int _currentPythonLogLevel = _getLogLevelValue(Logger::LEVEL_INFO);
+
+ void _updateCurrentPythonLogLevel() {
+ PyGILState_STATE state = PyGILState_Ensure();
+
+ try {
+ _currentPythonLogLevel = py::call_method<int>(_pyLogger, "getEffectiveLevel");
+ } catch (py::error_already_set e) {
+ PyErr_Print();
+ }
+
+ PyGILState_Release(state);
+ };
+
+ int _getLogLevelValue(Level level) {
+ return 10 + (level * 10);
+ }
+
+ public:
+
+ LoggerWrapper(const std::string &logger, PyObject* pyLogger) : _logger(logger) {
+ _pyLogger = pyLogger;
+ Py_XINCREF(_pyLogger);
+
+ _updateCurrentPythonLogLevel();
+ }
+
+ LoggerWrapper(const LoggerWrapper& other) {
Review comment:
Initialize `_logger` from `other._logger`
--
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] Bklyn commented on a change in pull request #7713: implement python logger wrapper
Posted by GitBox <gi...@apache.org>.
Bklyn commented on a change in pull request #7713:
URL: https://github.com/apache/pulsar/pull/7713#discussion_r598818323
##########
File path: pulsar-client-cpp/python/pulsar/__init__.py
##########
@@ -403,6 +405,8 @@ def __init__(self, service_url,
Configure whether the Pulsar client validates that the hostname of the
endpoint, matches the common name on the TLS certificate presented by
the endpoint.
+ * `logger`:
+ Set a Python logger for this Pulsar client.
Review comment:
Perhaps spell out explicitly that this must be an instance of `logging.Logger`?
--
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] aahmed-se commented on pull request #7713: implement python logger wrapper
Posted by GitBox <gi...@apache.org>.
aahmed-se commented on pull request #7713:
URL: https://github.com/apache/pulsar/pull/7713#issuecomment-806868983
rebase this on master.
--
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] pinkavaj commented on a change in pull request #7713: implement python logger wrapper
Posted by GitBox <gi...@apache.org>.
pinkavaj commented on a change in pull request #7713:
URL: https://github.com/apache/pulsar/pull/7713#discussion_r490738010
##########
File path: pulsar-client-cpp/python/src/config.cc
##########
@@ -74,6 +74,103 @@ static ClientConfiguration& ClientConfiguration_setAuthentication(ClientConfigur
return conf;
}
+class LoggerWrapper: public Logger {
+ std::string _logger;
+ PyObject* _pyLogger;
+ int _currentPythonLogLevel = 10 + (Logger::LEVEL_INFO*10);
+
+ void _updateCurrentPythonLogLevel() {
+ PyGILState_STATE state = PyGILState_Ensure();
+
+ try {
+ _currentPythonLogLevel = py::call_method<int>(_pyLogger, "getEffectiveLevel");
+ } catch (py::error_already_set e) {
+ PyErr_Print();
+ }
+
+ PyGILState_Release(state);
+ };
+
+ public:
+
+ LoggerWrapper(const std::string &logger, PyObject* pyLogger) : _logger(logger) {
+ _pyLogger = pyLogger;
+ Py_XINCREF(_pyLogger);
+
+ _updateCurrentPythonLogLevel();
+ }
+
+ LoggerWrapper(const LoggerWrapper& other) {
+ _pyLogger = other._pyLogger;
+ Py_XINCREF(_pyLogger);
+ }
+
+ LoggerWrapper& operator=(const LoggerWrapper& other) {
+ _pyLogger = other._pyLogger;
+ Py_XINCREF(_pyLogger);
+ return *this;
+ }
+
+ virtual ~LoggerWrapper() {
+ Py_XDECREF(_pyLogger);
+ }
+
+ bool isEnabled(Level level) {
+ return 10 + (level*10) >= _currentPythonLogLevel;
Review comment:
This equiation for log level conversion is used at multiple places, consider wrap it into function or somethink like that.
----------------------------------------------------------------
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] Bklyn commented on a change in pull request #7713: implement python logger wrapper
Posted by GitBox <gi...@apache.org>.
Bklyn commented on a change in pull request #7713:
URL: https://github.com/apache/pulsar/pull/7713#discussion_r595179081
##########
File path: pulsar-client-cpp/python/src/config.cc
##########
@@ -74,6 +74,107 @@ static ClientConfiguration& ClientConfiguration_setAuthentication(ClientConfigur
return conf;
}
+class LoggerWrapper: public Logger {
+ std::string _logger;
+ PyObject* _pyLogger;
+ int _currentPythonLogLevel = _getLogLevelValue(Logger::LEVEL_INFO);
+
+ void _updateCurrentPythonLogLevel() {
+ PyGILState_STATE state = PyGILState_Ensure();
+
+ try {
+ _currentPythonLogLevel = py::call_method<int>(_pyLogger, "getEffectiveLevel");
+ } catch (py::error_already_set e) {
+ PyErr_Print();
+ }
+
+ PyGILState_Release(state);
+ };
+
+ int _getLogLevelValue(Level level) {
+ return 10 + (level * 10);
+ }
+
+ public:
+
+ LoggerWrapper(const std::string &logger, PyObject* pyLogger) : _logger(logger) {
+ _pyLogger = pyLogger;
+ Py_XINCREF(_pyLogger);
+
+ _updateCurrentPythonLogLevel();
+ }
+
+ LoggerWrapper(const LoggerWrapper& other) {
+ _pyLogger = other._pyLogger;
+ Py_XINCREF(_pyLogger);
+ }
+
+ LoggerWrapper& operator=(const LoggerWrapper& other) {
+ _pyLogger = other._pyLogger;
+ Py_XINCREF(_pyLogger);
+ return *this;
+ }
+
+ virtual ~LoggerWrapper() {
+ Py_XDECREF(_pyLogger);
+ }
+
+ bool isEnabled(Level level) {
+ return _getLogLevelValue(level) >= _currentPythonLogLevel;
+ }
+
+ void log(Level level, int line, const std::string& message) {
Review comment:
Not having this info only truly matters if the Python `Logger` is connected ultimately to one or more `Formatter`s that has tokens like `filename`, `pathname` or `lineno` in its format string.
It _should_ be possible to create a `LogRecord` object (https://docs.python.org/3/library/logging.html#logrecord-objects) and pass it to `Logger.handle` method instead of using the simpler `info`, `warning` etc methods. I for one think it is not worth the trouble and the implementation is fine as-is.
----------------------------------------------------------------
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] mianos commented on pull request #7713: implement python logger wrapper
Posted by GitBox <gi...@apache.org>.
mianos commented on pull request #7713:
URL: https://github.com/apache/pulsar/pull/7713#issuecomment-824604840
Does anyone know the why the unit test run was cancelled?
This PR would be great to get into our unloved python client.
--
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 a change in pull request #7713: implement python logger wrapper
Posted by GitBox <gi...@apache.org>.
lbenc135 commented on a change in pull request #7713:
URL: https://github.com/apache/pulsar/pull/7713#discussion_r599459309
##########
File path: pulsar-client-cpp/python/pulsar/__init__.py
##########
@@ -403,6 +405,8 @@ def __init__(self, service_url,
Configure whether the Pulsar client validates that the hostname of the
endpoint, matches the common name on the TLS certificate presented by
the endpoint.
+ * `logger`:
+ Set a Python logger for this Pulsar client.
Review comment:
Fixed
--
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 a change in pull request #7713: implement python logger wrapper
Posted by GitBox <gi...@apache.org>.
lbenc135 commented on a change in pull request #7713:
URL: https://github.com/apache/pulsar/pull/7713#discussion_r599459545
##########
File path: pulsar-client-cpp/python/src/config.cc
##########
@@ -88,6 +88,107 @@ static ProducerConfiguration& ProducerConfiguration_setCryptoKeyReader(ProducerC
return conf;
}
+class LoggerWrapper: public Logger {
+ std::string _logger;
+ PyObject* _pyLogger;
+ int _currentPythonLogLevel = _getLogLevelValue(Logger::LEVEL_INFO);
+
+ void _updateCurrentPythonLogLevel() {
+ PyGILState_STATE state = PyGILState_Ensure();
+
+ try {
+ _currentPythonLogLevel = py::call_method<int>(_pyLogger, "getEffectiveLevel");
+ } catch (py::error_already_set e) {
+ PyErr_Print();
+ }
+
+ PyGILState_Release(state);
+ };
+
+ int _getLogLevelValue(Level level) {
+ return 10 + (level * 10);
+ }
+
+ public:
+
+ LoggerWrapper(const std::string &logger, PyObject* pyLogger) : _logger(logger) {
+ _pyLogger = pyLogger;
+ Py_XINCREF(_pyLogger);
+
+ _updateCurrentPythonLogLevel();
+ }
+
+ LoggerWrapper(const LoggerWrapper& other) {
+ _pyLogger = other._pyLogger;
+ Py_XINCREF(_pyLogger);
+ }
+
+ LoggerWrapper& operator=(const LoggerWrapper& other) {
Review comment:
You were right, it was unused. Removed it.
--
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] pinkavaj commented on a change in pull request #7713: implement python logger wrapper
Posted by GitBox <gi...@apache.org>.
pinkavaj commented on a change in pull request #7713:
URL: https://github.com/apache/pulsar/pull/7713#discussion_r562163882
##########
File path: pulsar-client-cpp/python/src/config.cc
##########
@@ -74,6 +74,107 @@ static ClientConfiguration& ClientConfiguration_setAuthentication(ClientConfigur
return conf;
}
+class LoggerWrapper: public Logger {
+ std::string _logger;
+ PyObject* _pyLogger;
+ int _currentPythonLogLevel = _getLogLevelValue(Logger::LEVEL_INFO);
+
+ void _updateCurrentPythonLogLevel() {
+ PyGILState_STATE state = PyGILState_Ensure();
+
+ try {
+ _currentPythonLogLevel = py::call_method<int>(_pyLogger, "getEffectiveLevel");
+ } catch (py::error_already_set e) {
+ PyErr_Print();
+ }
+
+ PyGILState_Release(state);
+ };
+
+ int _getLogLevelValue(Level level) {
+ return 10 + (level * 10);
+ }
+
+ public:
+
+ LoggerWrapper(const std::string &logger, PyObject* pyLogger) : _logger(logger) {
+ _pyLogger = pyLogger;
+ Py_XINCREF(_pyLogger);
+
+ _updateCurrentPythonLogLevel();
+ }
+
+ LoggerWrapper(const LoggerWrapper& other) {
+ _pyLogger = other._pyLogger;
+ Py_XINCREF(_pyLogger);
+ }
+
+ LoggerWrapper& operator=(const LoggerWrapper& other) {
+ _pyLogger = other._pyLogger;
+ Py_XINCREF(_pyLogger);
+ return *this;
+ }
+
+ virtual ~LoggerWrapper() {
+ Py_XDECREF(_pyLogger);
+ }
+
+ bool isEnabled(Level level) {
+ return _getLogLevelValue(level) >= _currentPythonLogLevel;
+ }
+
+ void log(Level level, int line, const std::string& message) {
Review comment:
Feel free to show example and you incentive to help with decission.
----------------------------------------------------------------
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] codelipenghui merged pull request #7713: implement python logger wrapper
Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #7713:
URL: https://github.com/apache/pulsar/pull/7713
--
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 a change in pull request #7713: implement python logger wrapper
Posted by GitBox <gi...@apache.org>.
lbenc135 commented on a change in pull request #7713:
URL: https://github.com/apache/pulsar/pull/7713#discussion_r499136007
##########
File path: pulsar-client-cpp/python/src/config.cc
##########
@@ -74,6 +74,103 @@ static ClientConfiguration& ClientConfiguration_setAuthentication(ClientConfigur
return conf;
}
+class LoggerWrapper: public Logger {
+ std::string _logger;
+ PyObject* _pyLogger;
+ int _currentPythonLogLevel = 10 + (Logger::LEVEL_INFO*10);
+
+ void _updateCurrentPythonLogLevel() {
+ PyGILState_STATE state = PyGILState_Ensure();
+
+ try {
+ _currentPythonLogLevel = py::call_method<int>(_pyLogger, "getEffectiveLevel");
+ } catch (py::error_already_set e) {
+ PyErr_Print();
+ }
+
+ PyGILState_Release(state);
+ };
+
+ public:
+
+ LoggerWrapper(const std::string &logger, PyObject* pyLogger) : _logger(logger) {
+ _pyLogger = pyLogger;
+ Py_XINCREF(_pyLogger);
+
+ _updateCurrentPythonLogLevel();
+ }
+
+ LoggerWrapper(const LoggerWrapper& other) {
+ _pyLogger = other._pyLogger;
+ Py_XINCREF(_pyLogger);
+ }
+
+ LoggerWrapper& operator=(const LoggerWrapper& other) {
+ _pyLogger = other._pyLogger;
+ Py_XINCREF(_pyLogger);
+ return *this;
+ }
+
+ virtual ~LoggerWrapper() {
+ Py_XDECREF(_pyLogger);
+ }
+
+ bool isEnabled(Level level) {
+ return 10 + (level*10) >= _currentPythonLogLevel;
Review comment:
extracted into a function
----------------------------------------------------------------
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