Re: [Django] #18978: Move cleanup management command to contrib.sessions

[Django]

#18978: Move cleanup management command to contrib.sessions
-+-
 Reporter:  rasca|Owner:  aaugustin
 Type:   |   Status:  closed
  Cleanup/optimization   |  Version:  master
Component:  Core (Management |   Resolution:  fixed
  commands)  | Triage Stage:  Accepted
 Severity:  Normal   |  Needs documentation:  0
 Keywords:   |  Patch needs improvement:  0
Has patch:  1|UI/UX:  0
  Needs tests:  1|
Easy pickings:  1|
-+-
Changes (by Aymeric Augustin ):

 * status:  new => closed
 * resolution:   => fixed


Comment:

 In [changeset:"b760503a270484679ad0e06b73e3ed417a113221"]:
 {{{
 #!CommitTicketReference repository=""
 revision="b760503a270484679ad0e06b73e3ed417a113221"
 [1.5.x] Fixed #18978 -- Moved cleanup command to sessions.

 This removes a dependency of 'core' on 'contrib'.

 Backport of 83ba0a9 from master.

 This deprecation occurs after the alpha, but it's a prerequisite
 for fixing decently #18194 which is a release blocker.
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #18978: Move cleanup management command to contrib.sessions

[Django]

#18978: Move cleanup management command to contrib.sessions
-+-
 Reporter:  rasca|Owner:  aaugustin
 Type:   |   Status:  new
  Cleanup/optimization   |  Version:  master
Component:  Core (Management |   Resolution:
  commands)  | Triage Stage:  Accepted
 Severity:  Normal   |  Needs documentation:  0
 Keywords:   |  Patch needs improvement:  0
Has patch:  1|UI/UX:  0
  Needs tests:  1|
Easy pickings:  1|
-+-

Comment (by aaugustin):

 In fact `daily_cleanup.py` should have been removed in #5522, like `make-
 messages.py` and `compile-messages.py`.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #18978: Move cleanup management command to contrib.sessions

[Django]

#18978: Move cleanup management command to contrib.sessions
-+-
 Reporter:  rasca|Owner:  aaugustin
 Type:   |   Status:  new
  Cleanup/optimization   |  Version:  master
Component:  Core (Management |   Resolution:
  commands)  | Triage Stage:  Accepted
 Severity:  Normal   |  Needs documentation:  0
 Keywords:   |  Patch needs improvement:  0
Has patch:  1|UI/UX:  0
  Needs tests:  1|
Easy pickings:  1|
-+-

Comment (by aaugustin):

 The discussion has diverged from the original report :/

 `cleanup` is a dependency of `core` on `contrib`, and such dependencies
 shouldn't exist. This means me must move the `cleanup` command to
 `django.contrib.sessions` and give it a sessions-specific name.
 `clearsessions` will do. Can we limit the scope of this ticket to this
 change? I have a patch ready for this.

 There hasn't been an obvious need for a more generic cleanup mechanism
 over the last five years. Designing one would be a new feature. This
 should be done separately from this bugfix. (IMHO it's much better to keep
 separate management commands because they may profit from being run at
 different times, on different servers, etc.)

 It isn't useful to provide two different ways to achieve exactly the same
 effect; for this reason `daily_cleanup.py` should be removed rather than
 documented.

 Finally, there's the question of clearing expired sessions in backends
 other than the database backend. This means pushing the clean up logic
 inside the backends, like the latest pull request does. This doesn't make
 sense with the cookie backend — data is stored by the clients. This isn't
 doable with the cache backend because we can't iterate over all keys —
 cache might handle its own expiration. This only leaves the file backend,
 which is tracked by #18194.

 Note that there are two different goals in clearing expired sessions:
   - freeing storage space;
   - prevent an attacker from re-using a session indefinitely — this is
 probably why #18194 was classified as a release blocker.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #18978: Move cleanup management command to contrib.sessions

[Django]

#18978: Move cleanup management command to contrib.sessions
-+-
 Reporter:  rasca|Owner:  aaugustin
 Type:   |   Status:  new
  Cleanup/optimization   |  Version:  master
Component:  Core (Management |   Resolution:
  commands)  | Triage Stage:  Accepted
 Severity:  Normal   |  Needs documentation:  0
 Keywords:   |  Patch needs improvement:  0
Has patch:  1|UI/UX:  0
  Needs tests:  1|
Easy pickings:  1|
-+-
Changes (by aaugustin):

 * owner:  rasca => aaugustin


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #18978: Move cleanup management command to contrib.sessions

[Django]

#18978: Move cleanup management command to contrib.sessions
-+-
 Reporter:  rasca|Owner:  rasca
 Type:   |   Status:  new
  Cleanup/optimization   |  Version:  master
Component:  Core (Management |   Resolution:
  commands)  | Triage Stage:  Accepted
 Severity:  Normal   |  Needs documentation:  0
 Keywords:   |  Patch needs improvement:  0
Has patch:  1|UI/UX:  0
  Needs tests:  1|
Easy pickings:  1|
-+-

Comment (by Elvard):

 Hi crodjer,
 you've included only
 https://code.djangoproject.com/attachment/ticket/18194/sessions-cleanup-
 files-1.patch which is not enough as I mentioned before:

 > Alright, I've found, that file-based sessions doesn't support deletion
 of expired files and patch above doesn't provided it either. It is in
 another patch from #18194,
 https://code.djangoproject.com/attachment/ticket/18194/files-sessions-
 server-expiry-3.patch. However, I can't agree with it's implementation.
 See related ticket for more details.

 I've already applied that patch to this commit
 
https://github.com/elvard/django/commit/969822aa8b51af7840d5809b649101b291d26623

 As I said, it doesn't work, since file-based sessions doesn't have any
 "expiration" mechanism. It's included in the other patch, but I can't
 agree with it's implementation, so I opened discussion at #18194.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #18978: Move cleanup management command to contrib.sessions

[Django]

#18978: Move cleanup management command to contrib.sessions
-+-
 Reporter:  rasca|Owner:  rasca
 Type:   |   Status:  new
  Cleanup/optimization   |  Version:  master
Component:  Core (Management |   Resolution:
  commands)  | Triage Stage:  Accepted
 Severity:  Normal   |  Needs documentation:  0
 Keywords:   |  Patch needs improvement:  0
Has patch:  1|UI/UX:  0
  Needs tests:  1|
Easy pickings:  1|
-+-
Changes (by crodjer):

 * cc: crodjer@… (added)


Comment:

 The changes which the linked patches from #18194 bring in are updated in a
 pull request I have posted over github:
 https://github.com/django/django/pull/452. I also mentioned this ticket
 over there.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #18978: Move cleanup management command to contrib.sessions

[Django]

#18978: Move cleanup management command to contrib.sessions
-+-
 Reporter:  rasca|Owner:  rasca
 Type:   |   Status:  new
  Cleanup/optimization   |  Version:  master
Component:  Core (Management |   Resolution:
  commands)  | Triage Stage:  Accepted
 Severity:  Normal   |  Needs documentation:  0
 Keywords:   |  Patch needs improvement:  0
Has patch:  1|UI/UX:  0
  Needs tests:  1|
Easy pickings:  1|
-+-

Comment (by Elvard):

 Alright, I've found, that file-based sessions doesn't support deletion of
 expired files and patch above doesn't provided it either. It is in another
 patch from #18194, https://code.djangoproject.com/attachment/ticket/18194
 /files-sessions-server-expiry-3.patch. However, I can't agree with it's
 implementation. See related ticket for more details.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #18978: Move cleanup management command to contrib.sessions

[Django]

#18978: Move cleanup management command to contrib.sessions
-+-
 Reporter:  rasca|Owner:  rasca
 Type:   |   Status:  new
  Cleanup/optimization   |  Version:  master
Component:  Core (Management |   Resolution:
  commands)  | Triage Stage:  Accepted
 Severity:  Normal   |  Needs documentation:  0
 Keywords:   |  Patch needs improvement:  0
Has patch:  1|UI/UX:  0
  Needs tests:  1|
Easy pickings:  1|
-+-
Changes (by Elvard):

 * needs_tests:  0 => 1


Comment:

 It's there, but right now is cleanup method implemented for database and
 file based session store only. There're also missing tests for file based
 clean up method…

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #18978: Move cleanup management command to contrib.sessions

[Django]

#18978: Move cleanup management command to contrib.sessions
-+-
 Reporter:  rasca|Owner:  rasca
 Type:   |   Status:  new
  Cleanup/optimization   |  Version:  master
Component:  Core (Management |   Resolution:
  commands)  | Triage Stage:  Accepted
 Severity:  Normal   |  Needs documentation:  0
 Keywords:   |  Patch needs improvement:  0
Has patch:  1|UI/UX:  0
  Needs tests:  0|
Easy pickings:  1|
-+-

Comment (by ptone):

 refs #18194

 we should probably integrate the work of the following patch into this
 work - which isn't directly related to the ticket it's attached to.

 https://code.djangoproject.com/attachment/ticket/18194/sessions-cleanup-
 files-1.patch

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #18978: Move cleanup management command to contrib.sessions

[Django]

#18978: Move cleanup management command to contrib.sessions
-+-
 Reporter:  rasca|Owner:  rasca
 Type:   |   Status:  new
  Cleanup/optimization   |  Version:  master
Component:  Core (Management |   Resolution:
  commands)  | Triage Stage:  Accepted
 Severity:  Normal   |  Needs documentation:  0
 Keywords:   |  Patch needs improvement:  0
Has patch:  1|UI/UX:  0
  Needs tests:  0|
Easy pickings:  1|
-+-
Changes (by Elvard):

 * needs_better_patch:  1 => 0
 * needs_tests:  1 => 0
 * needs_docs:  1 => 0


Comment:

 Sorry for delay,
 new updates were added to pull request, documentation and tests.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #18978: Move cleanup management command to contrib.sessions

[Django]

#18978: Move cleanup management command to contrib.sessions
-+-
 Reporter:  rasca|Owner:  rasca
 Type:   |   Status:  new
  Cleanup/optimization   |  Version:  master
Component:  Core (Management |   Resolution:
  commands)  | Triage Stage:  Accepted
 Severity:  Normal   |  Needs documentation:  1
 Keywords:   |  Patch needs improvement:  1
Has patch:  1|UI/UX:  0
  Needs tests:  1|
Easy pickings:  1|
-+-

Comment (by ptone):

 So I'm going to retract the whole idea of a "special" pattern for cleanup
 - as pointed by Aymeric in IRC and myself in comment 1 - there hasn't
 proven to be a big need for a generic cleanup routine, and implementing
 this pattern would be a case of YAGNI design.

 New tests are still needed - preferably inside contrib.sessions, not in
 admin_scripts where I would have expected the old ones to be, but I agree
 - this doesn't seem to be currently tested.

 There is also a tiny cleanup needed in bin/daily-cleanup.py

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #18978: Move cleanup management command to contrib.sessions

[Django]

#18978: Move cleanup management command to contrib.sessions
-+-
 Reporter:  rasca|Owner:  rasca
 Type:   |   Status:  new
  Cleanup/optimization   |  Version:  master
Component:  Core (Management |   Resolution:
  commands)  | Triage Stage:  Accepted
 Severity:  Normal   |  Needs documentation:  1
 Keywords:   |  Patch needs improvement:  1
Has patch:  1|UI/UX:  0
  Needs tests:  1|
Easy pickings:  1|
-+-

Comment (by Elvard):

 The pull is there: https://github.com/django/django/pull/420

 And yes, I assume that future implementation of cleanup would require more
 discussion. I'll leave it for now.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #18978: Move cleanup management command to contrib.sessions

[Django]

#18978: Move cleanup management command to contrib.sessions
-+-
 Reporter:  rasca|Owner:  rasca
 Type:   |   Status:  new
  Cleanup/optimization   |  Version:  master
Component:  Core (Management |   Resolution:
  commands)  | Triage Stage:  Accepted
 Severity:  Normal   |  Needs documentation:  1
 Keywords:   |  Patch needs improvement:  1
Has patch:  1|UI/UX:  0
  Needs tests:  1|
Easy pickings:  1|
-+-

Comment (by ptone):

 Sorry about the confusion on pull requests - it is an area for growth as
 we transition to github.

 I agree about not deprecating cleanup - it can remain as a simple alias
 for now.

 I do think we pull it from the documentation entirely for now (but see
 ramble below)

 Any new features of a future implementation of the cleanup command are
 beyond the scope of this particular ticket, and yes - would require its
 own ticket.  However I can tell you that the approach of using a setting
 is probably a non-starter.  Perhaps allow any app to define a 'cleanup'
 command - and the core cleanup command would use the command discovery
 machinery to find and run each one of them.  However that will take some
 design consideration, as it is an entirely new behavior for management
 commands, and becomes perhaps a bit too implicit/magical.  If this were
 the approach taken, the sessions cleanup - would once again be called
 cleanup - and so perhaps these issues should be coupled. In otherwords,
 make that decision first.

 So to rephrase, cleanup would become a new "special" management command -
 which will simply call the cleanup command in any app, and the current
 sessions cleanup command would be one such example located in
 contrib.sessions.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #18978: Move cleanup management command to contrib.sessions

[Django]

#18978: Move cleanup management command to contrib.sessions
-+-
 Reporter:  rasca|Owner:  rasca
 Type:   |   Status:  new
  Cleanup/optimization   |  Version:  master
Component:  Core (Management |   Resolution:
  commands)  | Triage Stage:  Accepted
 Severity:  Normal   |  Needs documentation:  1
 Keywords:   |  Patch needs improvement:  1
Has patch:  1|UI/UX:  0
  Needs tests:  1|
Easy pickings:  1|
-+-
Changes (by Elvard):

 * cc: tomas.ehrlich@… (added)


-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #18978: Move cleanup management command to contrib.sessions

[Django]

#18978: Move cleanup management command to contrib.sessions
-+-
 Reporter:  rasca|Owner:  rasca
 Type:   |   Status:  new
  Cleanup/optimization   |  Version:  master
Component:  Core (Management |   Resolution:
  commands)  | Triage Stage:  Accepted
 Severity:  Normal   |  Needs documentation:  1
 Keywords:   |  Patch needs improvement:  1
Has patch:  1|UI/UX:  0
  Needs tests:  1|
Easy pickings:  1|
-+-

Comment (by Elvard):

 Ok, it was easier for me to pull request rather than submit patch here…

 I just don't agree with any deprecation warning. The '''cleanup''' command
 do the same job as it always did -- it cleans expired sessions and it
 requires Sessions installed. I don't think it will be removed in future.
 Each app will probably provide it's own '''clean*''' method and
 '''cleanup''' will collect them.

 Possible feature in this scenario would be new setting, where one could
 define list of all '''clean*''' commands triggered by '''cleanup'''
 command. This would be handy, eg: django-registration provides
 '''cleanupregistration'''. This would be quiet easily implemented. Should
 I post a patch for it or open new ticket for this feature?

 Last thing, I just can't find any tests for old '''cleanup''' command. I
 have to probably write them…

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #18978: Move cleanup management command to contrib.sessions

[Django]

#18978: Move cleanup management command to contrib.sessions
-+-
 Reporter:  rasca|Owner:  rasca
 Type:   |   Status:  new
  Cleanup/optimization   |  Version:  master
Component:  Core (Management |   Resolution:
  commands)  | Triage Stage:  Accepted
 Severity:  Normal   |  Needs documentation:  1
 Keywords:   |  Patch needs improvement:  1
Has patch:  1|UI/UX:  0
  Needs tests:  1|
Easy pickings:  1|
-+-
Changes (by ptone):

 * needs_better_patch:  0 => 1
 * version:  1.4 => master
 * needs_tests:  0 => 1
 * needs_docs:  0 => 1


Comment:

 This is a great start - the patch will need docs (mostly a move of current
 docs), including an item in the release notes, and tests - the test can
 probably be just the old tests pointing to the new command. Rather than
 raise a deprecation warning - the old cleanup command could output a
 terminal message (in red) about the move.

 Pull requests should generally be opened for complete patches, ready for
 final review.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #18978: Move cleanup management command to contrib.sessions

[Django]

#18978: Move cleanup management command to contrib.sessions
-+-
 Reporter:  rasca|Owner:  rasca
 Type:   |   Status:  new
  Cleanup/optimization   |  Version:  1.4
Component:  Core (Management |   Resolution:
  commands)  | Triage Stage:  Accepted
 Severity:  Normal   |  Needs documentation:  0
 Keywords:   |  Patch needs improvement:  0
Has patch:  1|UI/UX:  0
  Needs tests:  0|
Easy pickings:  1|
-+-
Changes (by Elvard):

 * has_patch:  0 => 1


Comment:

 Fixed in pull https://github.com/django/django/pull/418

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #18978: Move cleanup management command to contrib.sessions

[Django]

#18978: Move cleanup management command to contrib.sessions
-+-
 Reporter:  rasca|Owner:  rasca
 Type:   |   Status:  new
  Cleanup/optimization   |  Version:  1.4
Component:  Core (Management |   Resolution:
  commands)  | Triage Stage:  Accepted
 Severity:  Normal   |  Needs documentation:  0
 Keywords:   |  Patch needs improvement:  0
Has patch:  0|UI/UX:  0
  Needs tests:  0|
Easy pickings:  1|
-+-
Changes (by ptone):

 * needs_better_patch:   => 0
 * stage:  Unreviewed => Accepted
 * needs_tests:   => 0
 * needs_docs:   => 0


Comment:

 I think the note about "(only expired sessions at the moment)" has always
 left the door open for other non-sessions related cleanup.  But this
 hasn't proven to be the case in 4-5 years, and it could be argued that any
 future cleanup should not be coupled to sessions.

 I'd be moderately in favor of making the command name more explicit,
 perhaps "clearsessions", with a deprecation for the old one.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.