Logger with Admin Client #1758
Merged
Logger with Admin Client #1758
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
anchitj
reviewed
Jun 20, 2024
| """ | ||
| Create a new AdminClient using the provided configuration dictionary. | ||
|
|
||
| The AdminClient is a standard Kafka protocol client, supporting | ||
| the standard librdkafka configuration properties as specified at | ||
| https://github.com/edenhill/librdkafka/blob/master/CONFIGURATION.md | ||
|
|
||
| :param dict config: Configuration properties. At a minimum ``bootstrap.servers`` **should** be set\n" |
There was a problem hiding this comment.
Shouldn't it be named conf? And also the trailing \n can be removed
| """ | ||
| Create a new AdminClient using the provided configuration dictionary. | ||
|
|
||
| The AdminClient is a standard Kafka protocol client, supporting | ||
| the standard librdkafka configuration properties as specified at | ||
| https://github.com/edenhill/librdkafka/blob/master/CONFIGURATION.md | ||
|
|
||
| :param dict config: Configuration properties. At a minimum ``bootstrap.servers`` **should** be set\n" | ||
| :param Logger logger: Optional Logger instance to use as a custom log messages handler. | ||
|
|
||
| At least 'bootstrap.servers' should be configured. |
There was a problem hiding this comment.
This can be removed as it's included above now
tests/test_log.py
Outdated
Comment on lines
187
to
192
| stringBuffer = StringIO() | ||
| logger = logging.getLogger('Admin') | ||
| logger.setLevel(logging.DEBUG) | ||
| handler = logging.StreamHandler(stringBuffer) | ||
| handler.setFormatter(logging.Formatter('%(name)s Logger | %(message)s')) | ||
| logger.addHandler(handler) |
There was a problem hiding this comment.
This is being repeated here and the above two functions. Can be moved to setup_logger or some helper function
examples/adminapi.py
Outdated
| @@ -920,3 +928,6 @@ def example_list_offsets(a, args): | |||
| sys.exit(1) | |||
|
|
|||
| opsmap[operation](a, args) | |||
|
|
|||
| # Log messages through custom logger if provided | |||
| a.poll(0) | |||
tests/test_log.py
Outdated
| logMessage = stringBuffer.getvalue().strip() | ||
| stringBuffer.close() | ||
|
|
||
| assert "Admin Logger | INIT" in logMessage |
emasab
requested changes
Jun 20, 2024
pranavrth
force-pushed
the
dev_fix-logger-arg-admin-client
branch
from
5479623
to
a2be27b
Compare
emasab
requested changes
Jun 20, 2024
emasab
requested changes
Jun 20, 2024
There is inbetween manipulation of the config properties in python binding as well. Due to that part, I have to check here first. |
* Updated examples/adminapi.py to include usage of the custom logger with AdminClient
pranavrth
force-pushed
the
dev_fix-logger-arg-admin-client
branch
from
53aac04
to
5abfee1
Compare
emasab
requested changes
Jul 5, 2024
CHANGELOG.md
Outdated
| - Removed usage of `strcpy` to enhance security of the client (#1745) | ||
| - Fixed invalid write in `OAUTHBEARER/OIDC` extensions copy (#1745) | ||
| - Fixed an issue related to import error of `TopicCollection` and `TopicPartitionInfo` classes when importing through other module like mypy. | ||
| - Fixed a segfault when `commit` or `store_offsets` consumer method is called incorrectly with errored Message object | ||
| - Fixed `logger` not working when provided as an argument to `AdminClient` |
There was a problem hiding this comment.
To clarify it was possible before:
Suggested change
| - Fixed `logger` not working when provided as an argument to `AdminClient` | |
| - Fixed `logger` not working when provided as an argument to `AdminClient` instead of a configuration property. |
| # Create Admin client | ||
| a = AdminClient({'bootstrap.servers': broker, | ||
| 'debug': 'all'}, | ||
| logger=logger) |
There was a problem hiding this comment.
Suggested change
| logger=logger) | |
| logger=logger) | |
| # Alternatively, pass the logger as a key. | |
| # When passing it as an argument, it overwrites the key. | |
| # | |
| # a = AdminClient({'bootstrap.servers': broker, | |
| # 'debug': 'all', | |
| # 'logger': logger}) |
| @@ -4,10 +4,12 @@ | |||
|
|
|||
| v2.4.1 is a maintenance release with the following fixes and enhancements: | |||
There was a problem hiding this comment.
Version can be changed to 2.5.0 when we merge delete_records
There was a problem hiding this comment.
Already done that in delete_records as that is a new feature.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
loggernot working when provided as an argument toAdminClientexamples/adminapi.pyto include usage of the custom logger withAdminClient