Re: [Django] #9084: File-based session does not store any data on Windows
Django
------------------------------------+----------------------------------
Reporter: mizutori | Owner:
Type: Bug | Status: new
Milestone: | Component: contrib.sessions
Version: 1.3 | Severity: Normal
Resolution: | Keywords:
Triage Stage: Accepted | Has patch: 1
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 1 | Easy pickings: 0
UI/UX: 0 |
------------------------------------+----------------------------------
Changes (by anonymous):
* ui_ux: => 0
* version: 1.2 => 1.3
* easy: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/9084#comment:9>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
------------------------------------+----------------------------------
Reporter: mizutori | Owner:
Type: Bug | Status: new
Milestone: | Component: contrib.sessions
Version: 1.3 | Severity: Normal
Resolution: | Keywords:
Needs documentation: 0 | Needs tests: 1
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------+----------------------------------
Changes (by jezdez):
* needs_better_patch: 1 => 0
* has_patch: 1 => 0
* needs_tests: 0 => 1
Comment:
Please provide a proper patch. This also needs tests.
--
Ticket URL: <https://code.djangoproject.com/ticket/9084#comment:10>
Django
Reporter: mizutori | Owner:
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by anonymous):
This still happens in 1.4.1. Unfortunately it makes file-based sessions
useless on Django in Windows (i.e., development or whatever). Also, it's
hard to find, as the exception is caught without warning -- this wouldn't
be so bad if it broke noisily.
--
Ticket URL: <https://code.djangoproject.com/ticket/9084#comment:11>
Django
----------------------------------+------------------------------------
Reporter: mizutori | Owner:
Type: Bug | Status: new
Component: contrib.sessions | Version: 1.3
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+------------------------------------
Comment (by aaugustin):
[http://docs.python.org/library/os#os.rename os.rename] hasn't improved on
Windows. Would it make sense to use
[http://docs.python.org/library/shutil#shutil.copyfile shutil.copyfile]
instead when the platform is Windows?
This isn't atomic, and if I understand correctly, it may still be
vulnerable to exceptions if the destination is open for reading at the
wrong time.
But we can't do anything about that, besides telling people to use a
better OS — ie. POSIX-compliant.
--
Ticket URL: <https://code.djangoproject.com/ticket/9084#comment:12>
Django
----------------------------------+------------------------------------
Type: Bug | Status: assigned
Component: contrib.sessions | Version: 1.3
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+------------------------------------
* keywords: => sprint2013
* owner: => joeri
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/9084#comment:13>
Django
Reporter: mizutori | Owner: joeri
Type: Bug | Status: assigned
Component: contrib.sessions | Version: 1.3
Severity: Normal | Resolution:
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by joeri):
* has_patch: 0 => 1
* needs_tests: 1 => 0
* stage: Accepted => Ready for checkin
Comment:
I made a pull request https://github.com/django/django/pull/823 that
changes `os.rename` to `shutil.move` which does (unlike the Python
documentation indicates) an atomic `os.rename` but falls back to "the best
Windows got to an atomic rename" in this specific case: `shutil.copy2` and
`os.unlink`.
The file based session save function is actually tested and fails on
Windows:
{{{
(env) D:\Work\Projects\joeri\django\tests>python runtests.py
--settings=test_sqlite sessions
Creating test database for alias 'default'...
Creating test database for alias 'other'...
........................................................................................x........................................................................................F................................
======================================================================
FAIL: test_clearsessions_command
(django.contrib.sessions.tests.FileSessionTests)
----------------------------------------------------------------------
Traceback (most recent call last):
File "D:\Work\Projects\joeri\django\django\test\utils.py", line 227, in
inner
return test_func(*args, **kwargs)
File "D:\Work\Projects\joeri\django\django\contrib\sessions\tests.py",
line 444, in test_clearsessions_command
self.assertEqual(1, count_sessions())
AssertionError: 1 != 2
----------------------------------------------------------------------
Ran 210 tests in 0.276s
FAILED (failures=1, expected failures=1)
Destroying test database for alias 'default'...
Destroying test database for alias 'other'...
}}}
After patch:
{{{
(env) D:\Work\Projects\joeri\django\tests>python runtests.py
--settings=test_sqlite sessions
Creating test database for alias 'default'...
Creating test database for alias 'other'...
........................................................................................x.........................................................................................................................
----------------------------------------------------------------------
Ran 210 tests in 0.235s
OK (expected failures=1)
Destroying test database for alias 'default'...
Destroying test database for alias 'other'...
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/9084#comment:14>
Django
-------------------------------------+-------------------------------------
Reporter: mizutori | Owner: joeri
Component: contrib.sessions | Version: 1.3
Severity: Normal | Resolution: fixed
Keywords: sprint2013 | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"b9cc61021a0db1e5b41e61d3e53180e4fc618f9c"]:
{{{
#!CommitTicketReference repository=""
revision="b9cc61021a0db1e5b41e61d3e53180e4fc618f9c"
Fixed #9084 - Best approach for an OS to atomically rename the session
file.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/9084#comment:15>
Django
-------------------------------------+-------------------------------------
Reporter: mizutori | Owner: joeri
Component: contrib.sessions | Version: 1.5
Severity: Normal | Resolution:
Keywords: sprint2013 | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* version: 1.3 => 1.5
* resolution: fixed =>
Comment:
Django 1.5.1 still not fix
--
Ticket URL: <https://code.djangoproject.com/ticket/9084#comment:16>
Django
-------------------------------------+-------------------------------------
Reporter: mizutori | Owner: joeri
Component: contrib.sessions | Version: 1.5
Severity: Normal | Resolution: fixed
Keywords: sprint2013 | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
It's fixed in master.
--
Ticket URL: <https://code.djangoproject.com/ticket/9084#comment:17>
Django
-------------------------------------+-------------------------------------
Reporter: mizutori | Owner: joeri
Type: Bug | Status: closed
Component: contrib.sessions | Version: 1.5
Severity: Normal | Resolution: fixed
Keywords: sprint2013 | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by anonymous):
How to solve it? Django 1.5.1 still not fix
--
Ticket URL: <https://code.djangoproject.com/ticket/9084#comment:18>
Django
-------------------------------------+-------------------------------------
Reporter: mizutori | Owner: joeri
Type: Bug | Status: closed
Component: contrib.sessions | Version: 1.5
Severity: Normal | Resolution: fixed
Keywords: sprint2013 | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by anonymous):
Confirmed, still not working in 1.5.1.
--
Ticket URL: <https://code.djangoproject.com/ticket/9084#comment:19>
Django
-------------------------------------+-------------------------------------
Reporter: mizutori | Owner: joeri
Component: contrib.sessions | Version: 1.5
Severity: Normal | Resolution:
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
* status: closed => new
* resolution: fixed =>
--
Ticket URL: <https://code.djangoproject.com/ticket/9084#comment:20>
Django
-------------------------------------+-------------------------------------
Reporter: mizutori | Owner: joeri
Component: contrib.sessions | Version: 1.5
Severity: Normal | Resolution: fixed
Keywords: sprint2013 | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
The problem is fixed in
[https://github.com/django/django/commit/b9cc61021a0db1e5b41e61d3e53180e4fc618f9c
master and 1.6a1].
It won't be backported to 1.5.X since
[https://docs.djangoproject.com/en/1.5/internals/release-process
/#supported-versions it's not considered critical].
--
Ticket URL: <https://code.djangoproject.com/ticket/9084#comment:21>
Django
-------------------------------------+-------------------------------------
Reporter: mizutori | Owner: joeri
Type: Bug | Status: closed
Component: contrib.sessions | Version: 1.5
Severity: Normal | Resolution: fixed
Keywords: sprint2013 | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by me21@…):
A new function is introduced in Python 3.3, os.replace(), which renames
files atomically and cross-platformly: http://bugs.python.org/issue8828
It might be possible to use this function in future, when Django support
for Python 2.x is abandoned, or when os.replace() is (ever) backported to
Python 2.x.
--
Ticket URL: <https://code.djangoproject.com/ticket/9084#comment:22>
Django
-------------------------------------+-------------------------------------
Reporter: mizutori | Owner: joeri
Type: Bug | Status: closed
Component: contrib.sessions | Version: 1.5
Severity: Normal | Resolution: fixed
Keywords: sprint2013 | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by me21@…):
In fact, since MoveFileEx API is considered to be atomic (for example,
Java uses it: http://bugs.python.org/issue8828#msg146307), Django could
use it through ctypes interface even now (https://github.com/mitsuhiko
/python-atomicfile/ - _rename() function - and disregard that its author
believes it to be non-atomic, it seems to be atomic actually). The
os.replace() function in Python 3.3 is implemented through MoveFileEx
anyway.
--
Ticket URL: <https://code.djangoproject.com/ticket/9084#comment:23>
Django
-------------------------------------+-------------------------------------
Reporter: mizutori | Owner: joeri
Type: Bug | Status: closed
Component: contrib.sessions | Version: 1.5
Severity: Normal | Resolution: fixed
Keywords: sprint2013 | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by me21@…):
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/9084#comment:24>