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

circuits.web.Sessions should have an expire method #133

Closed
mnlipp opened this Issue Oct 2, 2015 · 3 comments

Comments

Projects
None yet
3 participants
@mnlipp
Contributor

mnlipp commented Oct 2, 2015

The policy, when to expire a session, is, of course, application dependent. But Sessions should have a method expire(session) that removes the session from the dict. Currently, you have to access the Sessions object's _data attribute, which violates the python conventions.

As an alternative, the class' comment could point out that sessions don't expire and encourage users to develop and use session managers derived from Sessions that implement a policy for expiration (and can access _data "legally", depending on whether you consider an _attribute in python private or protected). Of course, from a designer's point of view, it would still be better to provide a method.

@prologic

This comment has been minimized.

Show comment
Hide comment
@prologic

prologic Oct 2, 2015

Member

Seems like adding .expire() would be a good idea :)

Solidifying a base class and API for all derived session management would
be good :)

Let's add this as a new issue; seems it would be trivial to add this?

cheers
James

James Mills / prologic

E: prologic@shortcircuit.net.au
W: prologic.shortcircuit.net.au

On Fri, Oct 2, 2015 at 3:22 AM, mnlipp notifications@github.com wrote:

The policy, when to expire a session, is, of course, application
dependent. But Sessions should have a method expire(session) that removes
the session from the dict. Currently, you have to access the Sessions
object's _data attribute, which violates the python conventions.

As an alternative, the class' comment could point out that sessions don't
expire and encourage users to develop and use session managers derived from
Sessions that implement a policy for expiration (and can access _data
"legally", depending on whether you consider an _attribute in python
private or protected). Of course, from a designer's point of view, it would
still be better to provide a method.


Reply to this email directly or view it on GitHub
#133.

Member

prologic commented Oct 2, 2015

Seems like adding .expire() would be a good idea :)

Solidifying a base class and API for all derived session management would
be good :)

Let's add this as a new issue; seems it would be trivial to add this?

cheers
James

James Mills / prologic

E: prologic@shortcircuit.net.au
W: prologic.shortcircuit.net.au

On Fri, Oct 2, 2015 at 3:22 AM, mnlipp notifications@github.com wrote:

The policy, when to expire a session, is, of course, application
dependent. But Sessions should have a method expire(session) that removes
the session from the dict. Currently, you have to access the Sessions
object's _data attribute, which violates the python conventions.

As an alternative, the class' comment could point out that sessions don't
expire and encourage users to develop and use session managers derived from
Sessions that implement a policy for expiration (and can access _data
"legally", depending on whether you consider an _attribute in python
private or protected). Of course, from a designer's point of view, it would
still be better to provide a method.


Reply to this email directly or view it on GitHub
#133.

@prologic prologic added the enhancement label Oct 2, 2015

@prologic prologic added this to the 3.2 milestone Oct 2, 2015

@prologic prologic added the ready label Oct 3, 2015

@prologic prologic modified the milestones: 3.2, 3.3 Jun 2, 2016

@eriol

This comment has been minimized.

Show comment
Hide comment
@eriol

eriol Jul 24, 2016

Member

@mnlipp you were talking about something like this a0bdf7e? Only exposing a method to not use _data directly, or something more/different?

Member

eriol commented Jul 24, 2016

@mnlipp you were talking about something like this a0bdf7e? Only exposing a method to not use _data directly, or something more/different?

@mnlipp

This comment has been minimized.

Show comment
Hide comment
@mnlipp

mnlipp Jul 24, 2016

Contributor

Yes, this is all I meant to address.

Contributor

mnlipp commented Jul 24, 2016

Yes, this is all I meant to address.

prologic added a commit that referenced this issue Jan 23, 2017

@prologic prologic self-assigned this Jan 23, 2017

@prologic prologic added in progress and removed ready labels Jan 23, 2017

prologic added a commit that referenced this issue Jan 23, 2017

prologic added a commit that referenced this issue Jan 29, 2017

prologic added a commit that referenced this issue Jan 29, 2017

@prologic prologic closed this in #212 Jun 1, 2017

prologic added a commit that referenced this issue Jun 1, 2017

[web]: Improves the API for session management and adds expire suppor…
…t. (#212)

* [web]: Improve the API for session management and add expire support.

Fixes #133

* Address @spaceone's feedback

* wtf? Fixed Import order :)

* use circuits.six

* isort
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment