Skip to content

[Issue 180] Fix python2 x python3 incompatibility #225

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.

Sign up for GitHub

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

Jump to bottom
Open
almostimplemented wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[Issue 180] Fix python2 x python3 incompatibility #225

almostimplemented wants to merge 2 commits into from

Conversation

@almostimplemented
Copy link

@almostimplemented almostimplemented commented Oct 2, 2017

Addresses bug #180 encountered when running Graphite in Python 3.

@brutasse
Copy link
Owner

brutasse commented Oct 19, 2017

Would it be possible to add a regression test? I'm not sure which conditions triggers this issue.

@almostimplemented
Copy link
Author

almostimplemented commented Nov 1, 2017

Sure, added.

The condition to trigger is when the operands of the compare in bisect.bisect_left (specifically: if a[mid] < x: lo = mid+1) agree on the first value of the tuple, thus requiring a comparison of the next entry. In the current code, that entry is None, which is no longer a supported operand for comparison in Python3.

almostimplemented
almostimplemented commented Nov 2, 2017
View reviewed changes
tests/test_carbonlink.py
("127.0.0.1",None)
]
carbonlink = ConsistentHashRing(hosts)
node = carbonlink.get_node('hosts.worker44.cpu')
Copy link
Author

@almostimplemented almostimplemented Nov 2, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Hm, it looks like this passed the automated test... I arrived upon this value by iterating through metric values (e.g. for x in range(100): ... 'hosts.worker%s.cpu' % x ... ). This was the first value triggering the raised bisect exception.

@RincewindsHat
Copy link

RincewindsHat commented Apr 22, 2025

This seems to work for me.
To make this better findable, this is the error message I got :

TypeError: '<' not supported between instances of 'tuple' and 'NoneType'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@almostimplemented @brutasse @RincewindsHat