-
-
Notifications
You must be signed in to change notification settings - Fork 231
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Force event to str in ConsoleRenderer #221
Conversation
Hm yeah this is a regression. It should be pulled out and protected using an |
Ping? |
Sorry, I haven't had a chance to work on this. |
I'd be happy to close this for now and reopen once py2 support has been dropped? |
Python 2 hasn’t been dropped for structlog and I don’t plan on it anytime soon because I need it myself in the foreseeable future. 😇 We’ll only be removing 3.4 from CI in the next release. |
@hynek will this get merged anytime soon? I'm finding myself the need to log in a dict but be renderable as as string in development for pretty output. |
@magicalbanana not speaking for @hynek , but unlikely, since it doesn't support py2. I'd be happy to close since I probably won't have time to make it robustly py2 compatible soon, unless someone else wants to pick this up? |
I think this might be much easier than thought since structlog is depending on |
Possibly? Though I think that would subtly change the behavior on py2, because that would force event etc to be unicode strings, whereas currently they'll be byte strings (assuming that's what |
I keep forgetting how to push to other people's branches, so here's my suggested solution: diff --git src/structlog/dev.py src/structlog/dev.py
index 42bb10a..372f7fc 100644
--- src/structlog/dev.py
+++ src/structlog/dev.py
@@ -8,7 +8,7 @@ Helpers that make development with ``structlog`` more pleasant.
from __future__ import absolute_import, division, print_function
-from six import StringIO
+from six import PY2, StringIO, string_types
try:
@@ -214,7 +214,10 @@ class ConsoleRenderer(object):
)
# force event to str for compatibility with standard library
- event = str(event_dict.pop("event"))
+ event = event_dict.pop("event")
+ if not PY2 or not isinstance(event, string_types):
+ event = str(event)
+
if event_dict:
event = _pad(event, self._pad_event) + self._styles.reset + " "
else:
diff --git tests/test_dev.py tests/test_dev.py
index 8da5a15..5e0b0c2 100644
--- tests/test_dev.py
+++ tests/test_dev.py
@@ -91,6 +91,17 @@ class TestConsoleRenderer(object):
assert unpadded == rv
+ @pytest.mark.skipif(not six.PY2, reason="Problem only exists on Python 2.")
+ @pytest.mark.parametrize("s", [u"\xc3\xa4".encode("utf-8"), u"ä", "ä"])
+ def test_event_py2_only_stringify_non_strings(self, cr, s, styles):
+ """
+ If event is a string type already, leave it be on Python 2. Running
+ str() on unicode strings with non-ascii characters raises an error.
+ """
+ rv = cr(None, None, {"event": s})
+
+ assert styles.bright + s + styles.reset == rv
+
def test_level(self, cr, styles, padded):
"""
Levels are rendered aligned, in square brackets, and color coded.
Does that make sense? I'm honestly a bit surprised that StringIO makes this work but I'll take it I guess… |
Thanks, yeah that seems reasonable. I've added your patch as is. |
Thanks! |
Resolves #186
The only thing I'm not sure about is how this should behave with unicode in py2.7?