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

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

[Issue 180] Fix python2 x python3 incompatibility #225

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b0c6180
[Issue 180] Fix python2 x python3 incompatibility
Oct 2, 2017
635b42b
Add repro test case
Nov 1, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions graphite_api/carbonlink.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,15 +123,15 @@ def remove_node(self, key):
def get_node(self, key):
assert self.ring
position = self.compute_ring_position(key)
search_entry = position, None
search_entry = (position, )
index = bisect.bisect_left(self.ring, search_entry) % self.ring_len
entry = self.ring[index]
return entry[1]

def get_nodes(self, key):
nodes = []
position = self.compute_ring_position(key)
search_entry = position, None
search_entry = (position, )
index = bisect.bisect_left(self.ring, search_entry) % self.ring_len
last_index = (index - 1) % self.ring_len
nodes_len = len(nodes)
Expand Down
8 changes: 8 additions & 0 deletions tests/test_carbonlink.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,14 @@ def test_chr_get_nodes(self):
]
self.assertEqual(node, expected)

def test_chr_break_get_node(self):
hosts = [
("127.0.0.1",None)
]
carbonlink = ConsistentHashRing(hosts)
node = carbonlink.get_node('hosts.worker44.cpu')
Copy link
Copy Markdown
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.

self.assertEqual(node, ('127.0.0.1', None))


class ConsistentHashRingTestFNV1A(TestCase):
def test_chr_compute_ring_position_fnv1a(self):
Expand Down