Skip to content

Dbal session storage #2182

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

Merged
merged 3 commits into from
Oct 7, 2011
Merged

Dbal session storage #2182

merged 3 commits into from
Oct 7, 2011

Conversation

schmittjoh
Copy link
Contributor

Adds a session storage based on Doctrine DBAL.

@lsmith77
Copy link
Contributor

guess it would be nice to then provide a service inside the DoctrineBundle that reuses a global DoctrineDBAL connection, guess the connection to use would then need to be configured via the doctrine app config.

@schmittjoh
Copy link
Contributor Author

I haven't found a sane way to provide automatic configuration that's why I left this to the user to implement. It's also relatively easy:

services:
    dbal_session_storage:
        class: Symfony\Bridge\Doctrine\HttpFoundation\DbalSessionStorage
        arguments: [@database_connection]

framework:
    session: { storage_id: dbal_session_storage }

@stof
Copy link
Member

stof commented Sep 14, 2011

@lsmith77 There is an issue about reusing another DBAL connection: if the transaction is aborted by the ORM, the session could be aborted too as you cannot have 2 independent transactions AFAIK

@lsmith77
Copy link
Contributor

not sure how this is relevant. i mean why does the transaction need to be left open? just begin, write, commit ..

@stof
Copy link
Member

stof commented Sep 14, 2011

and what if the transaction of the ORM is still opened (let's say you use SimpleThingsTransactionalBundle) ?

@lsmith77
Copy link
Contributor

well thats a bit of an edge case imho. also i wonder if SimpleThingsTransactionalBundle shouldn't make sure that its transaction is closed before the session is written.

@schmittjoh
Copy link
Contributor Author

It closes them.

On Wed, Sep 14, 2011 at 11:06 PM, Lukas Kahwe Smith <
reply@reply.github.com>wrote:

well thats a bit of an edge case imho. also i wonder if
SimpleThingsTransactionalBundle shouldn't make sure that its transaction is
closed before the session is written.

Reply to this email directly or view it on GitHub:
#2182 (comment)

@stof
Copy link
Member

stof commented Sep 14, 2011

@schmittjoh Does it close them before writing the session ?

@schmittjoh
Copy link
Contributor Author

I think either commit, or rollback is called, but @beberlei can probably answer that better.

Anyway, it is not really related to this PR because it is up to the user which connection is used.

@stof
Copy link
Member

stof commented Sep 14, 2011

I know that one of them is called. But if they are called after the session is persisted to the DB, a rollback is an issue as it will rollback the session persistence as well.
The PR is indeed fine. What need to be changed is the doc about how to use it, to advocate using a separate connection instead of the default one (which is used in your example)

@schmittjoh
Copy link
Contributor Author

There is no doc yet, but lets see what @fabpot thinks of all of this first.

@fabpot
Copy link
Member

fabpot commented Sep 15, 2011

Any benefits over using the PDO session storage?

@lsmith77
Copy link
Contributor

cleaner code, potentially better support for niche RDBMS, centralized logging and finally afaik DoctrineDBAL has emulation for nested transactions.

@schmittjoh
Copy link
Contributor Author

The benefits (for me) are:

  • logging queries
  • better interoperability with existing build processes (migrations)
  • better database interoperability
  • re-using existing connection (I don't have the problem that Stof mentioned above)

@beberlei
Copy link
Contributor

The nested transactions is the problem here as @stof already said. If you reuse the default connection and use nested transactions that fail then this will make your session not save. That is why the docs should recommend you create a second connection for a dbal based session storage, even if it is using the same database. The PDO session storage would be a second connection besides DBAL aswell.

I like the migrations/schema hook though to create the table automatically through schema commands.

@fabpot
Copy link
Member

fabpot commented Sep 15, 2011

ok, looks good to me. Can you add some documentation about its usage (like the possible keys for options)? Is it possible to add some tests too?

@schmittjoh schmittjoh closed this Sep 22, 2011
@jdreesen
Copy link
Contributor

why did you close this?

@fabpot fabpot reopened this Sep 22, 2011
@schmittjoh
Copy link
Contributor Author

I can't put more time into this PR, however I'm using it for some time already, and there shouldn't be any major issues as it is basically copy/paste from the PDO session.

fabpot added a commit that referenced this pull request Oct 7, 2011
Commits
-------

3f8e8c9 fixes a session max lifetime handling
3abb7f3 fixed some conflicts with garbage collection
a1888b2 added a dbal session storage

Discussion
----------

Dbal session storage

Adds a session storage based on Doctrine DBAL.

---------------------------------------------------------------------------

by lsmith77 at 2011/09/14 13:39:28 -0700

guess it would be nice to then provide a service inside the DoctrineBundle that reuses a global DoctrineDBAL connection, guess the connection to use would then need to be configured via the doctrine app config.

---------------------------------------------------------------------------

by schmittjoh at 2011/09/14 13:42:34 -0700

I haven't found a sane way to provide automatic configuration that's why I left this to the user to implement. It's also relatively easy:

```yml
services:
    dbal_session_storage:
        class: Symfony\Bridge\Doctrine\HttpFoundation\DbalSessionStorage
        arguments: [@database_connection]

framework:
    session: { storage_id: dbal_session_storage }

---------------------------------------------------------------------------

by stof at 2011/09/14 13:57:48 -0700

@lsmith77 There is an issue about reusing another DBAL connection: if the transaction is aborted by the ORM, the session could be aborted too as you cannot have 2 independent transactions AFAIK

---------------------------------------------------------------------------

by lsmith77 at 2011/09/14 13:59:57 -0700

not sure how this is relevant. i mean why does the transaction need to be left open? just begin, write, commit ..

---------------------------------------------------------------------------

by stof at 2011/09/14 14:02:39 -0700

and what if the transaction of the ORM is still opened (let's say you use SimpleThingsTransactionalBundle) ?

---------------------------------------------------------------------------

by lsmith77 at 2011/09/14 14:06:12 -0700

well thats a bit of an edge case imho. also i wonder if SimpleThingsTransactionalBundle shouldn't make sure that its transaction is closed before the session is written.

---------------------------------------------------------------------------

by schmittjoh at 2011/09/14 14:06:56 -0700

It closes them.

On Wed, Sep 14, 2011 at 11:06 PM, Lukas Kahwe Smith <
reply@reply.github.com>wrote:

> well thats a bit of an edge case imho. also i wonder if
> SimpleThingsTransactionalBundle shouldn't make sure that its transaction is
> closed before the session is written.
>
> --
> Reply to this email directly or view it on GitHub:
> #2182 (comment)
>

---------------------------------------------------------------------------

by stof at 2011/09/14 14:15:02 -0700

@schmittjoh Does it close them **before** writing the session ?

---------------------------------------------------------------------------

by schmittjoh at 2011/09/14 14:44:15 -0700

I think either commit, or rollback is called, but @beberlei can probably answer that better.

Anyway, it is not really related to this PR because it is up to the user which connection is used.

---------------------------------------------------------------------------

by stof at 2011/09/14 14:58:48 -0700

I know that one of them is called. But if they are called after the session is persisted to the DB, a rollback is an issue as it will rollback the session persistence as well.
The PR is indeed fine. What need to be changed is the doc about how to use it, to advocate using a separate connection instead of the default one (which is used in your example)

---------------------------------------------------------------------------

by schmittjoh at 2011/09/15 02:57:34 -0700

There is no doc yet, but lets see what @fabpot thinks of all of this first.

---------------------------------------------------------------------------

by fabpot at 2011/09/15 04:57:57 -0700

Any benefits over using the PDO session storage?

---------------------------------------------------------------------------

by lsmith77 at 2011/09/15 05:00:50 -0700

cleaner code, potentially better support for niche RDBMS, centralized logging and finally afaik DoctrineDBAL has emulation for nested transactions.

---------------------------------------------------------------------------

by schmittjoh at 2011/09/15 05:11:00 -0700

The benefits (for me) are:

- logging queries
- better interoperability with existing build processes (migrations)
- better database interoperability
- re-using existing connection (I don't have the problem that Stof mentioned above)

---------------------------------------------------------------------------

by beberlei at 2011/09/15 06:18:22 -0700

The nested transactions is the problem here as @stof already said. If you reuse the default connection and use nested transactions that fail then this will make your session not save. That is why the docs should recommend you create a second connection for a dbal based session storage, even if it is using the same database. The PDO session storage would be a second connection besides DBAL aswell.

I like the migrations/schema hook though to create the table automatically through schema commands.

---------------------------------------------------------------------------

by fabpot at 2011/09/15 10:45:31 -0700

ok, looks good to me. Can you add some documentation about its usage (like the possible keys for options)? Is it possible to add some tests too?

---------------------------------------------------------------------------

by jdreesen at 2011/09/22 06:34:11 -0700

why did you close this?

---------------------------------------------------------------------------

by schmittjoh at 2011/09/30 06:26:12 -0700

I can't put more time into this PR, however I'm using it for some time already, and there shouldn't be any major issues as it is basically copy/paste from the PDO session.
@fabpot fabpot merged commit 3f8e8c9 into symfony:master Oct 7, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants