Python class w/ Telnet interface to memcached

Question

I haven't had anyone help me out with code review, etc, so I thought I'd post a Python class I put together for interfacing with Telnet to get information from a memcached server.

import re, telnetlib
class MemcachedStats:
 _client = None
 def __init__(self, host='localhost', port='11211'):
 self._host = host
 self._port = port
 @property
 def client(self):
 if self._client is None:
 self._client = telnetlib.Telnet(self._host, self._port)
 return self._client
 def key_details(self, sort=True):
 ' Return a list of tuples containing keys and details '
 keys = []
 slab_ids = self.slab_ids()
 for id in slab_ids:
 self.client.write("stats cachedump %s 100\n" % id)
 response = self.client.read_until('END')
 keys.extend(re.findall('ITEM (.*) \[(.*); (.*)\]', response))
 if sort:
 return sorted(keys)
 return keys
 def keys(self, sort=True):
 ' Return a list of keys in use '
 return [key[0] for key in self.key_details(sort=sort)]
 def slab_ids(self):
 ' Return a list of slab ids in use '
 self.client.write("stats items\n")
 response = self.client.read_until('END')
 return re.findall('STAT items:(.*):number', response)
 def stats(self):
 ' Return a dict containing memcached stats '
 self.client.write("stats\n")
 response = self.client.read_until('END')
 return dict(re.findall("STAT (.*) (.*)\r", response))

This is also up on GitHub.

I would love some feedback on:

• Organization
• Better ways of accomplishing the same result

Answer 1

The pattern

self.client.write("some command\n")
response = self.client.read_until('END')

appears three times in your code. I think this is often enough to warrant refactoring it into its own method like this:

def command(self, cmd):
 self.client.write("%s\n" % cmd)
 return self.client.read_until('END')

---

In key_details you're using extend to build up a list. However it's more pythonic to use list comprehensions than building up a list imperatively. Thus I'd recommend using the following list comprehension:

regex = 'ITEM (.*) \[(.*); (.*)\]'
cmd = "stats cachedump %s 100"
keys = [key for id in slab_ids for key in re.findall(regex, command(cmd % id))]

---

Afterwards you do this:

if sort:
 return sorted(keys)
return keys

Now this might be a matter of opinion, but I'd rather write this using an else:

if sort:
 return sorted(keys)
else:
 return keys

I think this is optically more pleasing as both returns are indented at the same level. It also makes it immediately obviously that the second return is what happens if the if condition is false.

• dlrust: thanks! I've incorporated some of these into the code on github. I also added precompiled versions of the regex into the class
• dlrust: Also, as far as the list comprehension goes. Since re.findall(regex, command(cmd % id)) can return multiple items, I am using extend. Is there a way to build a list that extends itself vs item by item building?
• sepp2k: @dlrust: Yes, of course, you need to add another for in the list comprehension to get a flat list. I actually meant to do that, but forgot somehow.
• dlrust: Thanks. Not sure if this is the best way to do this, but it works keys = [j for i in [self._key_regex.findall(self.command(cmd % id)) for id in self.slab_ids()] for j in i]
• sepp2k: @lrust: By using an inner list like that, you need one more for than you would otherwise. Does [key for id in slab_ids for key in self._key_regex.findall(self.command(cmd % id))] not work for you?