Adding Fix and Unit tests for UTF-8 Breakages#188
Adding Fix and Unit tests for UTF-8 Breakages#188Calvin Chan (CalvinChan1) wants to merge 9 commits into
Conversation
* Fixing encoding, change to utf8 from ascii * Adding unit tests * Remove imports * Adding unit test and fix for utf8 breakages * Remove print statement
|
Thanks for your pull request. Is this your first contribution to a Snowplow open source project? Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://github.com/snowplow/snowplow/wiki/CLA to learn more and sign. Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks. |
Ben Fradet (BenFradet)
left a comment
Ben Fradet (BenFradet)
left a comment
There was a problem hiding this comment.
LGTM overall, a couple of Qs though
| import logging | ||
|
|
||
| # Set default logging handler to avoid "No handler found" warnings. | ||
| logging.getLogger(__name__).addHandler(logging.NullHandler()) |
There was a problem hiding this comment.
why change the existing logging?
|
|
||
| except ImportError: | ||
| # Otherwise configure Celery with default settings | ||
| app = Celery("Snowplow", broker="redis://guest@localhost//") |
There was a problem hiding this comment.
what was the motivation behind this?
| json_dict = json.dumps(dict_, ensure_ascii=False) | ||
|
|
||
| if encode_base64: | ||
| encoded_dict = base64.urlsafe_b64encode(json_dict.encode("ascii")) |
There was a problem hiding this comment.
This PR doesn't solve the issue #194 in my local tests
Simply changing the encoding on this line to utf-8 does the trick
|
Closing due to long inactivity & missing CLA sign. This issue will be solved by #219 . |
7 participants
We’re trying to encode as ascii when the text is probably UTF-8 thus getting errors like:
UnicodeEncodeError: 'ascii' codec can't encode characters in position 133-141: ordinal not in range(128)