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

sessions documentation (client side vs server side) #434 #1888

Merged
merged 1 commit into from
Aug 28, 2016
Merged

sessions documentation (client side vs server side) #434 #1888

merged 1 commit into from
Aug 28, 2016

Conversation

dawranliou
Copy link
Contributor

@dawranliou dawranliou commented Jun 4, 2016

Mention the existence of Flask extentions that handle server-side sessions


This change is Reviewable

@dawranliou
Copy link
Contributor Author

PR for #434

@untitaker
Copy link
Contributor

We should go into a lot more detail and actually list extensions that do this.

@untitaker
Copy link
Contributor

Also the text above is very explicit about where sessions get stored (cookies). It can't harm to restate for clarity, but as of now the reading flow is a bit awkward IMO

@untitaker untitaker self-assigned this Jun 4, 2016
@jeffwidman
Copy link
Contributor

jeffwidman commented Jun 6, 2016

We should go into a lot more detail and actually list extensions that do this.

@untitaker I'm hesitant to go into much detail here because this is the quickstart, so we want to keep it quick, especially when Flask's default client-side sessions suffice for most use cases. Primarily I thought we could add a sentence to make n00b web-devs aware of the existence of server-side sessions and they can google if they want more.

Also, I don't know what our policy is on recommending specific extensions, I'm generally a little hesitant because there's no guarantees about them being maintained. In particular for sessions, even when the extension is well-maintained, there are potential security implications if anything is amiss... for example mbr/flask-kvsession#25 (comment)

@untitaker
Copy link
Contributor

I overlooked that this was the quickstart. At some point sessions might have to get its own page, if we want to list extensions.

I still insist about the reading flow being bad. The paragraph above talks about sessions being stored entirely client side and this one opens with "Sessions are stored clientside" as if that wasn't already established.

@jeffwidman jeffwidman added the docs label Jun 7, 2016
@untitaker
Copy link
Contributor

Hey @dawran6, are you still interested in pushing this PR forward?

Mention the existence of Flask extentions that handle server-side
sessions.
Attempt to improve the reading flow.
@dawranliou
Copy link
Contributor Author

Hey @untitaker , @jeffwidman , sorry for taking so long. I only changed a bit on the transition words. What do you guys think?

@untitaker
Copy link
Contributor

LGTM, definetly an improvement. Thanks!

@untitaker untitaker merged commit 3d856e0 into pallets:master Aug 28, 2016
@untitaker untitaker removed the waiting label Aug 28, 2016
@dawranliou dawranliou deleted the sessions-doc branch May 22, 2017 21:41
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants