Opened 11 years ago
Closed 10 years ago
#5507 closed (fixed)
File bases session backend failing on Win32
Reported by: | dougn | Owned by: | Philippe Raoult |
---|---|---|---|
Component: | Contrib apps | Version: | master |
Severity: | Keywords: | sessions, file backend | |
Cc: | Triage Stage: | Ready for checkin | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | UI/UX: |
Description
The new file based Session backend (ticket #2066, changeset [6333]) fails on Win23 due to the SESSION_FILE_PATH defaulting to '/tmp/'
django/trunk/django/conf/global_settings.py should get the default using the environ for the temporary storage on Win32/OSX
Adding the following to your local test settings file will work as a temporary workaround:
SESSION_FILE_PATH = os.environ['TMP']
Test Failure:
====================================================================== FAIL: Doctest: django.contrib.sessions.tests ---------------------------------------------------------------------- Traceback (most recent call last): File "C:\django-sprint\trunk\django\test\_doctest.py", line 2169, in runTest raise self.failureException(self.format_failure(new.getvalue())) AssertionError: Failed doctest test for django.contrib.sessions.tests File "C:\django-sprint\trunk\django\contrib\sessions\tests.py", line 0, in tests ---------------------------------------------------------------------- File "C:\django-sprint\trunk\django\contrib\sessions\tests.py", line 35, in django.contrib.sessions.tests Failed example: file_session.exists(file_session.session_key) Expected: True Got: False ---------------------------------------------------------------------- Ran 165 tests in 291.149s FAILED (failures=1)
Attachments (2)
Change History (13)
comment:1 Changed 11 years ago by
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 Changed 11 years ago by
Component: | Unit test system → Contrib apps |
---|---|
Has patch: | set |
Owner: | changed from nobody to Philippe Raoult |
Status: | new → assigned |
comment:3 Changed 11 years ago by
I fear there might be a meta-bug in here. The failure to create the session file should have resulted in an error, not the test returning 'False' when checking to see if there is a session. The real error (which could be permission based, or even typo based), is lost somewhere. I see this as being the equivalent to not having the table created in teh database, which gives an appropriate backend exception.
comment:4 follow-up: 5 Changed 11 years ago by
Yeah, I agree. In the meanwhile, I suggest commiting this patch asap so that tests can continue to be run on Windows.
comment:5 Changed 11 years ago by
Replying to deadwisdom:
Yeah, I agree. In the meanwhile, I suggest commiting this patch asap so that tests can continue to be run on Windows.
Please commit it.
comment:6 Changed 10 years ago by
Triage Stage: | Accepted → Ready for checkin |
---|
comment:7 Changed 10 years ago by
gettempdir()'s return doesn't have the trailing slash(on linux at least it returns '/tmp') would this be a problem?
comment:8 Changed 10 years ago by
Needs documentation: | set |
---|---|
Patch needs improvement: | set |
Triage Stage: | Ready for checkin → Accepted |
- Imports don't belong in the middle of files. They go at the top.
- Instead of importing tempfile unconditionally in
settings.py
, when it's only needed for the file-based session backend, let's have that setting default toNone
which will mean "at use time, it will be converted to the tmp directory's location" and then we can pick the right value indjango.contrib.session.backends.file
. We try fairly hard to avoid importing unneeded modules that are only going to be used rarely and this is one of those cases.
comment:9 Changed 10 years ago by
Patch needs improvement: | unset |
---|
Followed malcolm's guidance.
Alex: no the backend uses os.path.join so it will be added as/if needed.
comment:10 Changed 10 years ago by
Needs documentation: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
comment:11 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
attached patch fixes the issue. Tested under windows and linux, but should be portable due to the use of tempfile.gettempdir().