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

Updated to make thread-safe in Django. #39

Closed
wants to merge 1 commit into from

Conversation

alexander-marquardt
Copy link

This works with Django, but I did not test it with the webapp framework.

You can see a description of the problem that this addresses here: http://stackoverflow.com/questions/14167213/app-engine-django-interleaving-multiple-requests-interfering-in-gae-sessions

@dound
Copy link
Owner

dound commented Jan 14, 2013

Thanks for the patch! I've removed some unrelated changes (whitespace changes, double import of logging, and the wrapper for session cleanup) and recommitted the changes to a new branch lexalink_fix. Can you please look it over?

Also, the unit tests didn't seem to pass for me on this branch. Do they work for you?

@alexander-marquardt
Copy link
Author

Hi, the changes look good, however I suspect that you could probably come up with a cleaner general-purpose solution since you understand the code inside and out .. whereas I was just trying to fix the problem for my particular case (and only for Django).

I hadn't run your unit-test, but I have it running (and working) in a production system that has thousands of sessions per day. Sorry about that.

I suspect that while the Dango application works fine, I probably broke the webapp implementation (SessionMiddleware) since I am now passing an extra parameter (current_session) into the my_start_response function. I am also returning an extra parameter (_current_session) from the call function in SessionMiddleware.

I don't know anything about webapp since I do all my development in Django, so my solution is not gemerally applicable... sorry about that..

@dound
Copy link
Owner

dound commented Jan 23, 2013

Ah, so it goes. I'll have to see if I can port it to webapp at some point. I'll leave your pull request open in the meantime.

@rfrp
Copy link

rfrp commented Aug 9, 2013

Hi!

i try to use this patch, but it give me this error:

INFO     2013-08-09 06:03:58,885 recording.py:665] Saved; key: __appstats__:038100, part: 46 bytes, full: 6328 bytes, overhead: 0.000 + 0.006; link: http://192.168.0.102:8000/_ah/stats/details?time=1376028238141
ERROR    2013-08-09 06:03:58,887 wsgi.py:235] 
Traceback (most recent call last):
  File "/home/roberto/herramientas/google_appengine_1.8.2/google/appengine/runtime/wsgi.py", line 224, in Handle
    for chunk in result:
  File "/home/roberto/herramientas/google_appengine_1.8.2/google/appengine/ext/appstats/recording.py", line 1276, in appstats_wsgi_wrapper
    result = app(environ, appstats_start_response)
  File "/home/roberto/PycharmProjects/proyecto/gaesessions/__init__.py", line 474, in __call__
    return (_current_session, self.app(environ, my_start_response))
  File "/home/roberto/herramientas/google_appengine_1.8.2/lib/django-1.2/django/core/handlers/wsgi.py", line 265, in __call__
    start_response(status, response_headers)
TypeError: my_start_response() takes at least 3 arguments (2 given)

Any clue?

Thanks a lot!

@alexander-marquardt
Copy link
Author

The patch was only tested with django ..

On Fri, Aug 9, 2013 at 1:13 AM, rfrp notifications@github.com wrote:

Hi!

i try to use this patch, but it give me this error:

INFO 2013-08-09 06:03:58,885 recording.py:665] Saved; key: appstats:038100, part: 46 bytes, full: 6328 bytes, overhead: 0.000 + 0.006; link: http://192.168.0.102:8000/_ah/stats/details?time=1376028238141
ERROR 2013-08-09 06:03:58,887 wsgi.py:235]
Traceback (most recent call last):
File "/home/roberto/herramientas/google_appengine_1.8.2/google/appengine/runtime/wsgi.py", line 224, in Handle
for chunk in result:
File "/home/roberto/herramientas/google_appengine_1.8.2/google/appengine/ext/appstats/recording.py", line 1276, in appstats_wsgi_wrapper
result = app(environ, appstats_start_response)
File "/home/roberto/PycharmProjects/proyecto/gaesessions/init.py", line 474, in call
return (_current_session, self.app(environ, my_start_response))
File "/home/roberto/herramientas/google_appengine_1.8.2/lib/django-1.2/django/core/handlers/wsgi.py", line 265, in call
start_response(status, response_headers)
TypeError: my_start_response() takes at least 3 arguments (2 given)

Any clue?

Thanks a lot!


Reply to this email directly or view it on GitHubhttps://github.com//pull/39#issuecomment-22377044
.

@rfrp
Copy link

rfrp commented Aug 12, 2013

I'm using django and gae...

@alexander-marquardt
Copy link
Author

Here is the version that I am using.

You can also find it in the Lexalink project on github

On Sun, Aug 11, 2013 at 8:21 PM, rfrp notifications@github.com wrote:

I'm using django and gae...


Reply to this email directly or view it on GitHubhttps://github.com//pull/39#issuecomment-22469380
.

@rfrp
Copy link

rfrp commented Aug 12, 2013

I found my error. I was using a config from one of the examples:

https://github.com/dound/gae-sessions/blob/master/demo-with-google-logins/appengine_config.py

Y eliminates this from my project and works perfect.
Thanks a lot!

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

Successfully merging this pull request may close these issues.

3 participants