L125: Python Abseil Logging Support #505
Conversation
Co-authored-by: Ashesh Vidyut <[email protected]>
sergiitk
left a comment
sergiitk
left a comment
There was a problem hiding this comment.
LGTM, but let's ask Mark to give this a look once the remaining feedback is addressed.
L125-python-absl-logging-support.md
Outdated
| > Note that if abseil fixes https://github.com/abseil/abseil-cpp/issues/1656, | ||
| > gRPC Core itself will be able to invoke absl::InitializeLog(), but until then | ||
| > this is a workaround. |
There was a problem hiding this comment.
Looking at the ticket - I don't think its point is to allow to "Core itself" .. "to invoke absl::InitializeLog()", but to "Make absl::InitializeLog safe to be called multiple times", making the call a no-op if already been called. We'll still have to call it from Cython.
I think we should remove this block.
There was a problem hiding this comment.
Discussed IRL, to be rephrased to make it clear the call may be moved to gRPC Core depending on how abseil/abseil-cpp#1656 is resoved.
| [L117][L117]. | ||
|
|
||
| After this change, gRPC python users started seeing the warning log - | ||
| `WARNING: All log messages before absl::InitializeLog() is called are written to STDERR`. |
There was a problem hiding this comment.
How do we reproduce this? I was only able to get it printed with export GRPC_VERBOSITY=INFO (which also prints out Log level INFO is not suitable for production. Prefer WARNING or ERROR. However if you see this message in a debug environment or test environment it is safe to ignore this message., or with export GRPC_TRACE=connectivity_state (or any other module).
There was a problem hiding this comment.
I was able to reproduce by setting GRPC_VERBOSITY=debug
L125-python-absl-logging-support.md
Outdated
| _initialize() | ||
| ``` | ||
|
|
||
| ### Behaviour with multiple imports of gRPC |
There was a problem hiding this comment.
| ### Behaviour with multiple imports of gRPC | |
| ### Behavior with multiple imports of gRPC |
There was a problem hiding this comment.
This is yet another Americal vs British English differences I suppose. I was using the British English spelling, however changed as suggested now :)
addressed the feedback. @markdroth PTAL and let us know your comments too before we open for public review. |
4 participants
Changes to add Abseil logging support in gRPC Python hence resolving grpc/grpc#38703.
Implementation PR: