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

Incorrect conversion of numeric return values #8

Closed
sris opened this issue Jan 29, 2010 · 5 comments
Closed

Incorrect conversion of numeric return values #8

sris opened this issue Jan 29, 2010 · 5 comments

Comments

@sris
Copy link

sris commented Jan 29, 2010

Currently all values will be converted using int or float_fn in _get_value if it's possible.
This is done regardless of what was actually put in.
This can lead to some hard to spot errors.

Example

v1 = "1234567" # E.g. hash values or something "non controllable"
v2 = "1ab3dc8"
type(v1), v1, type(v2), v2 => (<type 'str'>, '1234567', <type 'str'>, '1ab3dc8')

redis.Redis().set("a", v1)
redis.Redis().set("b", v2)

v1 = redis.Redis().get("a")
v2 = redis.Redis().get("b")
type(v1), v1, type(v2), v2 => (<type 'int'>, 1234567, <type 'unicode'>, u'1ab3dc8')

As you can see the returned values no longer match in type. It's not very intuitive that some values automatically gets converted while other doesn't.

I think it would be more appropriate to always return the non converted values, unless, if made possible, some other type where explicitly provided.

@razmataz
Copy link

See the HEAD of my branch where all number magic has been removed (and pickle support added).

@Bernie
Copy link

Bernie commented Feb 5, 2010

I've also noticed a failure in redis.py as a result of this automatic conversion:

r = redis.Redis()
r.set('1', 'blah')
r.keys('*')

will result in an AttributeError as the keys method assumes the data comes in as a string and attempts to split it by white space--

Traceback (most recent call last):
File "", line 1, in
File "redis.py", line 366, in keys
return self.send_command('KEYS %s\r\n' % pattern).split()
AttributeError: 'int' object has no attribute 'split'

@andymccurdy
Copy link
Contributor

I've pushed a new branch of redis-py called "newapi". It's a completely refactored client. There are numerous changes under the hood, including pipelining, a more consistent interface across commands, and removal of "magic value decoding".

I've made an effort to be backwards compatible where possible, issuing DeprecationWarnings for things that will go away in a future version. Hopefully this will give people time to upgrade their code. Nevertheless, there are several backwards incompatible changes, most notably that the Redis client no longer attempts to be "clever" about casting values to ints and floats if the data looks like a number. For example:

r = Redis()
r.set('foo', 1)
r.get('foo')
'1' # a string, not an integer

There are tests for all the commands and I should have a working set of tests for pipelining and connection pooling this evening.

If you have some spare cycles, it'd be awesome if you could give the new client a test drive and provide any feedback.

Thanks!
-andy

@sris
Copy link
Author

sris commented Feb 16, 2010

Great!
I will give it a spin, especially the pipe-lining.

-- J

@andymccurdy
Copy link
Contributor

This issue has been resolved in the 1.34 release of redis-py.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants