Skip to content
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

fix django2.0 #179

Merged
merged 14 commits into from
Aug 15, 2018
Merged
4 changes: 4 additions & 0 deletions ChangeLog.rst
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
Change Log
==========
New in draft
--------------------
* Change set_many and set_multi api return value. see [pr](https://github.com/pinterest/pymemcache/pull/179)

New in version 1.4.4
--------------------
* pypy3 to travis test matrix
Expand Down
10 changes: 4 additions & 6 deletions pymemcache/client/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -309,17 +309,14 @@ def set_many(self, values, expire=0, noreply=None):
self.default_noreply).

Returns:
If no exception is raised, always returns True. Otherwise all, some
or none of the keys have been successfully set. If noreply is True
then a successful return does not guarantee that any keys were
successfully set (just that the keys were successfully sent).
Empty list
Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't this now return a list of failed sets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thx! i fixed it.

"""

# TODO: make this more performant by sending all the values first, then
# waiting for all the responses.
for key, value in six.iteritems(values):
self.set(key, value, expire, noreply)
return True
return []

set_multi = set_many

Expand Down Expand Up @@ -930,7 +927,8 @@ def set(self, key, value, expire=0, noreply=None):

def set_many(self, values, expire=0, noreply=None):
with self.client_pool.get_and_release(destroy_on_fail=True) as client:
return client.set_many(values, expire=expire, noreply=noreply)
client.set_many(values, expire=expire, noreply=noreply)
return []

set_multi = set_many

Expand Down
10 changes: 5 additions & 5 deletions pymemcache/client/hash.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,13 +241,13 @@ def decr(self, key, *args, **kwargs):

def set_many(self, values, *args, **kwargs):
client_batches = {}
end = []
failed = []

for key, value in values.items():
client = self._get_client(key)

if client is None:
end.append(False)
failed.append(key)
continue

if client.server not in client_batches:
Expand All @@ -261,11 +261,11 @@ def set_many(self, values, *args, **kwargs):
new_args.insert(0, values)
result = self._safely_run_func(
client,
client.set_many, False, *new_args, **kwargs
client.set_many, values.keys(), *new_args, **kwargs
Copy link
Contributor

Choose a reason for hiding this comment

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

so this means, if any of these keys fail, we mark all as failed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks!
That's true. I will fix it.

)
end.append(result)
failed.extend(result)

return all(end)
return failed

set_multi = set_many

Expand Down
20 changes: 15 additions & 5 deletions pymemcache/test/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,13 +156,13 @@ def test_set_noreply(self):
def test_set_many_success(self):
client = self.make_client([b'STORED\r\n'])
result = client.set_many({b'key': b'value'}, noreply=False)
assert result is True
assert result == []

def test_set_multi_success(self):
# Should just map to set_many
client = self.make_client([b'STORED\r\n'])
result = client.set_multi({b'key': b'value'}, noreply=False)
assert result is True
assert result == []

def test_add_stored(self):
client = self.make_client([b'STORED\r', b'\n'])
Expand Down Expand Up @@ -602,7 +602,7 @@ def _set():
def test_set_many_socket_handling(self):
client = self.make_client([b'STORED\r\n'])
result = client.set_many({b'key': b'value'}, noreply=False)
assert result is True
assert result == []
assert client.sock.closed is False
assert len(client.sock.send_bufs) == 1

Expand Down Expand Up @@ -738,6 +738,11 @@ def _default_noreply_true(self, cmd, args, response):
result = getattr(client, cmd)(*args)
assert result is True

def _default_noreply_true_and_empty_list(self, cmd, args, response):
client = self.make_client(response, default_noreply=True)
result = getattr(client, cmd)(*args)
assert result == []

def test_default_noreply_set(self):
with pytest.raises(MemcacheUnknownError):
self._default_noreply_false(
Expand All @@ -749,7 +754,7 @@ def test_default_noreply_set_many(self):
with pytest.raises(MemcacheUnknownError):
self._default_noreply_false(
'set_many', ({b'key': b'value'},), [b'NOT_STORED\r\n'])
self._default_noreply_true(
self._default_noreply_true_and_empty_list(
'set_many', ({b'key': b'value'},), [b'NOT_STORED\r\n'])

def test_default_noreply_add(self):
Expand Down Expand Up @@ -854,6 +859,11 @@ def _default_noreply_true(self, cmd, args, response):
result = getattr(client, cmd)(*args)
assert result is True

def _default_noreply_true_and_empty_list(self, cmd, args, response):
client = self.make_client(response, default_noreply=True)
result = getattr(client, cmd)(*args)
assert result == []

def test_default_noreply_set(self):
with pytest.raises(MemcacheUnknownError):
self._default_noreply_false(
Expand All @@ -865,7 +875,7 @@ def test_default_noreply_set_many(self):
with pytest.raises(MemcacheUnknownError):
self._default_noreply_false(
'set_many', ({b'key': b'value'},), [b'NOT_STORED\r\n'])
self._default_noreply_true(
self._default_noreply_true_and_empty_list(
'set_many', ({b'key': b'value'},), [b'NOT_STORED\r\n'])

def test_default_noreply_add(self):
Expand Down
2 changes: 1 addition & 1 deletion pymemcache/test/test_client_hash.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ def test_no_servers_left_with_set_many(self):
)

result = client.set_many({'foo': 'bar'})
assert result is False
assert result == ['foo']

def test_no_servers_left_with_get_many(self):
from pymemcache.client.hash import HashClient
Expand Down
2 changes: 1 addition & 1 deletion pymemcache/test/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ def set(self, key, value, expire=0, noreply=True):
def set_many(self, values, expire=None, noreply=True):
for key, value in six.iteritems(values):
self.set(key, value, expire, noreply)
return True
return []

set_multi = set_many

Expand Down