Skip to content

[dbal]: make dbal connection config usable again #765

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 1 commit into from
Feb 14, 2019

Conversation

ssiergl
Copy link
Contributor

@ssiergl ssiergl commented Feb 14, 2019

right now user cannot declare connection options.

Reason is that always 'connection' => 'url' was set which dbal always prefers over other
connection settings. With this fix connection can be defined dbal compliant like this:

  connection:
    host: 'localhost'
    port: '3306'
    dbname: 'app'
    user: 'app'
    password: ''

'dsn only' works like before.

@makasim what do you think?

@ssiergl ssiergl force-pushed the master branch 3 times, most recently from e9a0e25 to 9b345b3 Compare February 14, 2019 09:40
@makasim
Copy link
Member

makasim commented Feb 14, 2019

This is nice, could you please add a test for this case to DbalConnectionFactoryConfigTest?

@ssiergl
Copy link
Contributor Author

ssiergl commented Feb 14, 2019

yeah right now working on a pimcore plugin and the connection variables are pretty long...so the dsn is nearly unreadable :-p

added config tests. Not tested...but should be ok

@ssiergl ssiergl force-pushed the master branch 2 times, most recently from 9990b73 to 82277d2 Compare February 14, 2019 11:34
@ssiergl
Copy link
Contributor Author

ssiergl commented Feb 14, 2019

tested all possible configurations...looks all well

right now user cannot declare connection options.

Reason is that always 'connection' => 'url' was set which dbal always prefers over other
connection settings. With this fix connection can be defined dbal compliant like this:

      connection:
        host: 'localhost'
        port: '3306'
        dbname: 'app'
        user: 'app'
        password: ''

'dsn only' works like before.

PS3-4:
 - added config tests

PS5:
 - if a fq dsn and connection is set dsn should be prefered

PS6:
 - better naming for dbname in tests
@makasim makasim merged commit f9ebc64 into php-enqueue:master Feb 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants