Skip to content
This repository has been archived by the owner on Jul 11, 2022. It is now read-only.

Remove support for non-ascii baggage keys; enable testing with Py 3.6 #154

Merged
merged 3 commits into from
Apr 16, 2018
Merged

Remove support for non-ascii baggage keys; enable testing with Py 3.6 #154

merged 3 commits into from
Apr 16, 2018

Conversation

yurishkuro
Copy link
Member

@yurishkuro yurishkuro commented Apr 16, 2018 •
edited
Loading

  • enable testing with Python 3.6
  • some fixes to the codec to handle unicode strings with Py3
  • a fix for thrift converter that broke Py3 compatibility in 3.8.0 release
  • skip crossdock unit tests when on Py3
  • remove tests for non-ascii characters in baggage keys - while those tests were passing with Py2, they didn't actually confirm that such baggage was readable on the receiving side, and it didn't really work. It's very hard to make it work because there's no way of knowing which encoding the strings are coming in from inbound request. This change officially removes support for non-ascii characters in baggage keys.

Fixes #151
Fixes #137
Fixes #59
Partially solves #22 (missing roundtrip test with Unicode)

Fix tests to work with Py 3.6
b14b97d 
Signed-off-by: Yuri Shkuro <ys@uber.com>
@coveralls
Copy link

coveralls commented Apr 16, 2018 •
edited
Loading

Coverage Status

Coverage decreased (-0.2%) to 95.372% when pulling 83c0503 on fixes-for-py3 into ca16098 on master.

Remove 3.7
3484910 
Signed-off-by: Yuri Shkuro <ys@uber.com>
This was referenced Apr 16, 2018
Closed
Closed
Closed
Add unicode characher
83c0503 
Signed-off-by: Yuri Shkuro <ys@uber.com>
black-adder
black-adder approved these changes Apr 16, 2018
View reviewed changes
tests/test_codecs.py
'trace-attr-key1': '%FF',
'trace-attr-key2-caf\xc3\xa9': 'caf%C3%A9',
'trace-attr-key3': 'caf%C3%A9',
'trace-attr-key4-caf\xc3\xa9': 'value',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't this still a useful testcase?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated the PR description: This change officially removes support for non-ascii characters in baggage keys.

@yurishkuro yurishkuro merged commit 4200bdb into master Apr 16, 2018
@yurishkuro yurishkuro deleted the fixes-for-py3 branch April 16, 2018 16:37
@yurishkuro yurishkuro changed the title Fix tests to work with Py 3.6 Remove support for non-ascii baggage keys; enable testing with Py 3.6 Apr 16, 2018
This was referenced Apr 16, 2018
Closed
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@deejay1 deejay1 deejay1 approved these changes

@black-adder black-adder black-adder approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Thrift encoding breaks in python3 3.8.0 doesn't report spans in Python 3 Support Python 3.x
4 participants
@yurishkuro @coveralls @deejay1 @black-adder