coto / gae-boilerplate

Google App Engine Boilerplate
https://dev-dot-sandengine.appspot.com/
Other
685 stars 189 forks source link

User creation fails with new SDK (1.7.6) #236

Closed topbonus closed 11 years ago

topbonus commented 11 years ago

Hey,

just realized that creating users is failing with an InvalidAuthId exception (old sdk works fine).

cheers and Thanks for the great software!

cloud-eagle commented 11 years ago

The problem happens when using SDK 1.7.6. When i try to register a new user name, it thrown an error on browser: "unxpected creating user...". And a nother one when go to admin page: "... there is something wrong in our end..." The SDK 1.7.5 works fine. Hope you fix those problems soon. Thank you very much.

alexsmx commented 11 years ago

Didn't have the time to check it deeply, but this worked for me:

Substituted this line in handlers.py on RegisterHandler maybe someone can get some insight on what the real error might be.

user_info = models.User.get_by_email(email)

user_info = models.User.query(models.User.email == email).get()

cloud-eagle commented 11 years ago

@alexsmx Your solution does not works. Thank you for taking time to this issue

alexsmx commented 11 years ago

Yeah, somehow it works sometimes and others it fails, I've replaced it by this other solution. Tested it a couple of times.

user_info = user[1]

I'm not sure if this has to do with this:

Ancestor queries, are strongly consistent; queries that

# span entity groups are only eventually consistent. If we omitted the
# ancestor from the query, there would be a slight chance that a User
# that had just been written would not show up in a query.

Sometimes the user query returned None, other times it returned correctly a user, so using user[1] would save us that step (querying), user[1] has the user info when user[0] is True

rchaber commented 11 years ago

I might have a temporary solution (literally):

import time

and then, inside RegisterHandler in handlers.py, before user_info, insert a delay. So it looks like this:

time.sleep(1)
user_info = models.User.get_by_email(email)

For some reason, the creation of a new user is not immediate to the point it can be retrieved one command later. Although the above fix works, it's not the right solution to the problem.

viafo commented 11 years ago

Using a sleep won't be guaranteed to work - the datastore is only eventually consistent, so it may still fail. You're just reducing the chances of failure.

I'm also using alexsmx's approach - I'm pretty sure that's the correct approach: user_info = user[1]

The create_user API purposely returns a tuple so we don't have to query the datastore again. As this avoids the query, it avoids the datastore consistency errors we're seeing.

NB: If you've changed the config to use your own user model, you may want to change all the model.User references to self.auth.store.user_model. I have a change pending that I need go back and check why the build tests failed for that, so the lib code removes that dependency.

michaelkariv commented 11 years ago

I have had the same problem. I spend whole day wrestling with it. I suspected misconfiguration first. GAEB uses mixed metaphor - in the same function it uses both configured model self.auth.store.user_model.create_user and direct reference to the model models.User.get_by_email But no, my config is ok.

The reason why it takes long time, as you might notice 1.7.6 has problems stopping at breakpoints in Eclipse debugger. Damn.

I discovered sleep solution. setting to 3 seconds worked for me. It is not a real solution of cause.

What I suspect is the way 1.7.6 works with the database. It is said to more correctly emulating GAE backend. My best guess that NoSQL databases have this wonderful quality of eventual consistency. It means if you write a row to DB, and read that row back, you gonna be fine... eventually. But if you write and immediately read, YMMV.

IMHO that we have this problem on the local database is a GOOD THING. We have just written a user into a huge distributed data table. It could take time until it is propagated to all the servers the data stores on. So immediately reading it back should indeed not necessarily work. The fact we have it now in our face is a good thing. It makes us think about what it is we do.

One solution is to loop up to X times sleeping Y seconds each and wait until we have row propagated and readback succeeding.

The other solution is to understand why do we need to read back what we have just now successfully created and put into the database. create_user() returned (True, user), so why don't we trust it? There is something wrong with the logic. I need to sit and think how to rewrite this, but rewrite I will.

If you guys have a good advice, and better yet a patch to the GAEB code, LMK.

ronw23 commented 11 years ago

I had a slightly longer comment, but this happens because of the eventually consistency model of the HRD. Doing a put via create_user immediately followed by a query via get_user_by_password is not guaranteed to succeed because the indexes have not been updated yet. I don't use GAEB but do use webapp-improved and my solution was to replace the get_user_by_password with

                self.auth.unset_session()
                self.auth.set_session(self.auth.store.user_to_dict(info))

where info is the second element returned from create_user

pmanacas commented 11 years ago

self.auth.unset_session() self.auth.set_session(self.auth.store.user_to_dict(info))

worked a charm for me :-) thanks

grrosegr commented 11 years ago

Still an issue on Appengine 1.8.1

Fixed by alexsms's comment