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

Add cookbook article for using MongoDB to store session data #5478

Merged

Conversation

stevenmusumeche
Copy link
Contributor

Q A
Doc fix? yes
New docs? yes
Applies to all
Fixed tickets none

@stevenmusumeche
Copy link
Contributor Author

This is my first contribution so please let me know if anything needs to be changed.

Symfony has a built-in solution for NoSQL database session storage called
:class:`Symfony\\Component\\HttpFoundation\\Session\\Storage\\Handler\\MongoDbSessionHandler`.
MongoDB is an open-source document database that provides high performance,
high availability, and automatic scaling. This article assumes that you have
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor correction: we don't use the so-called "serial comma", so the comma before the and word must be removed. Thanks.

@javiereguiluz
Copy link
Member

@stevenmusumeche thanks for this proposal and congratulations on your first contribution!

Right now there is a minor technical issue which is the reason why the latest doc build throwed this error:

Warning, treated as error:
/home/travis/build/symfony/symfony-docs/cookbook/configuration/mongodb_session_storage.rst::   
WARNING: document isn't included in any toctree

To solve this issue, you need to edit the cookbook/map.rst.inc file and add a link to your article. The syntax is the same that you'll find for the other articles in the config section of the cookbook. If you have any question, please ask.

Lastly, regarding the article itself, I like it and it's very easy to read! The only thing I didn't like is how convoluted is the configuration to use Mongo for storing sessions. But of course this is not a problem of this article.

@wouterj
Copy link
Member

wouterj commented Jul 3, 2015

Actually, the build error shown by @javiereguiluz is not fixed by updating map.rst.inc (while you should update that article as well). The error is fixed by adding this document to the list of documents in cookbook/configuration/index.rst.

@stevenmusumeche
Copy link
Contributor Author

@wouterj thank you for the XML and PHP code blocks, I have added them to the PR.

@stevenmusumeche
Copy link
Contributor Author

@javiereguiluz looks like the build passed this time, ready to be merged

@xabbuh
Copy link
Member

xabbuh commented Jul 4, 2015

I suggest to also add a reference to the new article in the session section (we do the same with the PDO session handler).

@stevenmusumeche
Copy link
Contributor Author

@xabbuh good idea, I added that and reworded the existing tip

@xabbuh
Copy link
Member

xabbuh commented Jul 4, 2015

@stevenmusumeche Good catch, I actually didn't remember that we had a notice in /cookbook/session/sessions_directory.rst too. Could you also add a reference to /cookbook/session/index.rst (that's file I initially had in mind).

@stevenmusumeche
Copy link
Contributor Author

I don't see a reference to PDO session handler on this page (http://symfony.com/doc/current/cookbook/session/index.html). Is that what you are referring to?

@xabbuh
Copy link
Member

xabbuh commented Jul 5, 2015

I'm sorry cause I pointed you to the wrong file. The reference Im talking about is in the Session section of the /cookbook/map.rst.inc file.

@stevenmusumeche
Copy link
Contributor Author

@xabbuh, just updated

avoid_session_start
/cookbook/configuration/pdo_session_storage
/cookbook/configuration/mongodb_session_storage
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two lines from the configuration section need to be removed. Sorry for the confusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xabbuh got it, sorry about that

@stevenmusumeche
Copy link
Contributor Author

@xabbuh anything else need to be done?

@xabbuh
Copy link
Member

xabbuh commented Jul 14, 2015

👍

arguments: [mongodb://%mongodb_host%:27017]
session.handler.mongo:
class: Symfony\Component\HttpFoundation\Session\Storage\Handler\MongoDbSessionHandler
arguments: [@mongo_client, %mongo.session.options%]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

% is a reserved first character in older versions of the Yaml spec. Unfortunately, the syntax highlighter used on symfony.com follows this spec and breaks. Can you please wrap it in double quotes, "%mongo.session.options%"?

@wouterj wouterj merged commit deccf85 into symfony:2.3 Jul 22, 2015
wouterj added a commit that referenced this pull request Jul 22, 2015
… data (stevenmusumeche)

This PR was merged into the 2.3 branch.

Discussion
----------

Add cookbook article for using MongoDB to store session data

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | yes
| Applies to    | all
| Fixed tickets | none

Commits
-------

deccf85 remove links from incorrect page
5f30b07 add reference to mongodb session storage to cookbook index
4238fcc add reference to PDO and MongoDB session handlers in session index
2faa2ac add information about adding an index to improve garbage collection performance
dde0ee1 add link to MongoDB cookbook article
d3ba62f move parameter configuration into separate section of the article
4048e97 fix formatting
52d89dc add xml and php configuration blocks
8e20ba4 add new cookbook article to index files
2e7757f Add cookbook article for using MongoDB to store session data
wouterj added a commit to wouterj/symfony-docs that referenced this pull request Jul 22, 2015
@wouterj
Copy link
Member

wouterj commented Jul 22, 2015

Great! As you already did such a great job, I just merged it in and created a new PR myself with the 2 little changes: #5554 Feel free to comment if you disagree with any of them.

@stevenmusumeche
Copy link
Contributor Author

@wouterj one small change, the field that should be indexed is expires_at

@xabbuh
Copy link
Member

xabbuh commented Jul 25, 2015

@stevenmusumeche Do you mean line 168?

@timglabisch
Copy link
Contributor

👍

@stevenmusumeche
Copy link
Contributor Author

@xabbuh yes line 168. I mistyped the field name that should be indexed for improved GC performance.

wouterj added a commit that referenced this pull request Aug 20, 2015
This PR was merged into the 2.3 branch.

Discussion
----------

[Cookbook][Session] fix default expiry field name

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | all
| Fixed tickets |

As @stevenmusumeche noted in #5478 (comment), the [default expiry field name is expires_at](https://github.com/symfony/symfony/blob/2.8/src/Symfony/Component/HttpFoundation/Session/Storage/Handler/MongoDbSessionHandler.php#L86).

Commits
-------

1e1a129 [Cookbook][Session] fix default expiry field name
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants