Redis session error - latest web2py master

[Jose C]

Greetings,

Attempting to enable Redis for sessions and get the following error when web2py attempts to retrieve the keys it set on first access.  (first page loads and the keys are set and can be seen in redis via redis-cli).

python 3.6.8
web2py: latest master downloaded from repo at 10:00 GMT, 2 Nov 2019.
redis: 5.0.4
python-redis:  3.3.11
os: ubuntu 18.04 LTS

Traceback (most recent call last):
 File "/home/jose/python-environments/winp_ca/lib/web2py/gluon/restricted.py", line 219, in restricted
 exec(ccode, environment)
 File "/home/jose/python-environments/winp_ca/lib/web2py/applications/winp_ca/models/0_init.py", line 56, in <module>
 session.connect(request, response, db = sessiondb)
 File "/home/jose/python-environments/winp_ca/lib/web2py/gluon/globals.py", line 971, in connect
 row = table(record_id, unique_key=unique_key)
 File "/home/jose/python-environments/winp_ca/lib/web2py/gluon/contrib/redis_session.py", line 117, in __call__
 row = q.select()
 File "/home/jose/python-environments/winp_ca/lib/web2py/gluon/contrib/redis_session.py", line 185, in select
 rtn = {to_native(k.decode): v for k, v in self.db.r_server.hgetall(key).items()}
 File "/home/jose/python-environments/winp_ca/lib/web2py/gluon/contrib/redis_session.py", line 185, in <dictcomp>
 rtn = {to_native(k.decode): v for k, v in self.db.r_server.hgetall(key).items()}
 File "/home/jose/python-environments/winp_ca/lib/web2py/gluon/packages/dal/pydal/_compat.py", line 136, in to_native
 return obj.decode(charset, errors)
AttributeError: 'builtin_function_or_method' object has no attribute 'decode'

I know there had been issues with web2py-redis (see: https://groups.google.com/forum/#!topic/web2py/0iU2wqSik0k) which were resolved but suspect  this is another issue.  Anyone got this working with py3 and the py-redis 3.3 library combo?

Thanks,

Jose

[Jose C]

So it appears to be a bug... adding () after the decode on line 185 of redis_session.py appears to resolve the cause of this traceback.

However another issue has cropped up...  every page hit results in a new session key being created which means they're unusable.  Changing sessions from redis to the default disk based sessions works as expected so it appears to be something related to the redis implementation.

Can anyone confirm whether they are using redis for sessions successfully in a configuration similar to the above?

Thx,

Jose

[Leonel Câmara]

We also got this bug when we tried to upgrade to the latest web2py and had to rollback. We were using the old 2.X pyredis and then tried with 3.X you get the same problem with both. Please report this issue in github, if no one fixes it we will probably end up fixing it as we also need this fix.

[Jose C]

On Wednesday, 6 November 2019 15:19:07 UTC, Leonel Câmara wrote:

We also got this bug when we tried to upgrade to the latest web2py and had to rollback. We were using the old 2.X pyredis and then tried with 3.X you get the same problem with both. Please report this issue in github, if no one fixes it we will probably end up fixing it as we also need this fix.

Hi Leonel,

Thanks for confirming, I was pulling my hair (what little is left) out with this one.  I can also confirm that it is present with the 2.x as well as 3.x versions of pyredis.  I'll report on github.

Regards,

Jose

EDIT: github issue created: https://github.com/web2py/web2py/issues/2274

[Jim S]

Same here.  I've reported and submitted a fix but it was rejected.  It worked for all of our use cases.

[Jim S]

Just wondering if there has  been any further work on this.

I'm working on converting all to python 3 but this is keeping us from moving right now.

-Jim

[Jim S]

I just messed with this a little bit and got past the login with python3, web2py and redis.

Here is the code, lines 185 thru 192 of redis_session.py:

            rtn = {to_native(k.decode()): v for k, v in self.db.r_server.hgetall(key).items()}
            if rtn:
                if self.unique_key:
                    # make sure the id and unique_key are correct
                    if rtn['unique_key'].decode() == to_native(self.unique_key):
                        rtn['update_record'] = self.update  # update record support
                    else:
                        rtn = None

Added the parens after the k.decode in the first line and added the .decode after rtn['unique_key'] in the 4th line.  I did not submit a PR because I don't understand what is going on here.  

self.db.r_server.hgetall(key).items returns a list of value pairs that the first line converts to a dict.  However, after the dict is built, all the values are in binary.  The 'unique_key' value needs needs to NOT be binary.  However, it appears as though the 'session_data' needs to be  binary.  

Should everything EXCEPT the session_data be converted from binary?  I'm hoping the maintainer of redis_session.py could chime in with what is expected so we can get this fixed.

But, at least I finally got past the login...

Thoughts?

-Jim

[Jose C]

Nice catch, Jim.... the .decode() on line 192.  It appears to be working as expected now (at least in my use case).

I am going to continue testing session functionality and will report if any further issues arise.

[Leonel Câmara]

I haven't tried it, but I think the best fix is not to make the decode work by putting the obviously missing parenthesis. The fix is simply to remove the .decode, because to_native should take care of it, so this:

rtn = {to_native(k.decode): v for k, v in self.db.r_server.hgetall(key).items()}

Would become

rtn = {to_native(k): v for k, v in self.db.r_server.hgetall(key).items()}

The second decode you used is also probably wrong although it may work with python 3. I would say it should be like this

if to_native(rtn['unique_key']) == to_native(self.unique_key):

If no one makes a pull request in the mean time, I'll probably have time to test this later in the month and do it.

[Jim S]

Do you know much about sessions?  Are the values all supposed to be binary?  The .decode on the 

if to_native(rtn['unique_key']) == to_native(self.unique_key):

was to convert it from binary to test against the string stored in self.unique_key.

Also, this fix isn't working in my python 2 installation.  I'm getting other errors there but haven't had time to dig through it enough.

But, I really appreciate you taking a look.  To me, this is a huge problem that needs to be resolved.  Or, we have to stop using redis for sessions or the web2py redis layer completely.

-Jim

[Jim S]

FWIW - I gave up on waiting.  Converted my sessions to use the database.

I use redis in other parts of my app within web2py and am having no troubles there.  

-Jim

[Massimo Di Pierro]

Did I drop the ball on this one? I do not see a PR

[Jim S]

I submitted one quite some time ago for Python 2 and python redis 2 and 3, but not for Python 3.  The fix I proposed here doesn't not work for Python 2.    While I'd prefer to put sessions in redis, they work perfectly well in the database.  Redis works just fine (any version) as long as you're not use the redis_session.py in the web2py redis integration.

Long story short, I do not have a PR that fixes for redis 2 and 3 and python 2 and 3.

Any feel for how many people are using redis for sessions?

-Jim

[Jose C]

Any feel for how many people are using redis for sessions?

+1 on on redis for sessions.  Use it for multiple apps although they're all older web2py versions and python 2 setups.  Upgrading one to python 3 and web2py 2.18.5 which is when the redis session functionality broke.  Jim's fix is working for me on python3 with the early Nov master of web2py (pre 2.19.0 it appears from changelog).  Sessions appear to be working as intended under this setup.

I think Leonel posted earlier that they're also using redis for sessions.

HTH,

[Massimo Di Pierro]

py4web does sessions in redis by default (if you have redis). Just saying.

[Jose C]

On Thursday, 28 November 2019 23:21:39 UTC, Massimo Di Pierro wrote:

py4web does sessions in redis by default (if you have redis). Just saying.

Yep and it looks promising but I don't have the guts to migrate a mature site with tens of thousands of users on to it yet - a bit too bleeding edge still.  Plus it looks like it will take a reasonable amount of refactoring an existing web2py app on py4web from my (limited) testing.  But next new app I need to build I'll definitely give it a try.

[Leonel Câmara]

py4web does sessions in redis by default (if you have redis). Just saying.

Massimo, this really doesn't help. Web2py had working redis sessions and right now, in master, it doesn't. Web2py should not break existing apps just because there's  py4web, there's no way people can all migrate to py4web immediately. 

I think fixing this and launching a new web2py version should have high priority.

[Massimo Di Pierro]

You are right. I misunderstood the issue. I read the thread again I thought this was a py3 only issue. Will look into this shortly.