search

cooperl22 / laravel-db2

laravel-db2 is a simple DB2 service provider for Laravel. It provides DB2 Connection by extending the Illuminate Database component of the laravel framework.
Other
59 stars 64 forks source link

Add 'name' to config array #30

Closed sarahkemp closed 7 years ago

sarahkemp commented 7 years ago

After updating to ~5.4, I got a new bug as a result of the 'name' attribute not being defined in my config array. My primary database connection is to a MySQL database on localhost, while my IBM i connection is on another host. When the name attribute is empty, and Laravel's Builder did a findOrNew() that resulted in a 'new', it made a new instance and set the connection name to the results of $this->query->getConnection()->getName() which was returning null because there was no 'name' attribute in the config array. This caused the new instance to use my default connection, which did not work.

YMMV - I am still on Laravel 5.3*, perhaps this has improved in newer versions, but it was a bit of a bear to track down since it only fails when findOrNew needs to create rather than find.

This pull request simply adds back the 'name' attribute in the readme. It looks like it was used in place of driverName at one point? It was removed in commit https://github.com/cooperl22/laravel-db2/commit/e8f17c13a7839717a9bc029ab57d7f6598b52b97. If you have more information about the undocumented standard laravel setting method in that commit, I would be interested to see it to make sure I'm diagnosing this correctly.

*Edit: my mistake, I am on 5.3, not 5.2

cooperl22 commented 7 years ago

Well actually if you're still on Laravel 5.2 you shouldn't use the version 5.4 of my package since this latter now follows Laravel framework versioning.

That said I've just released another version of the package. 'name' is not documented very well. By default in the framework the name given to default connections (mysql, pgsql, sqlite, sqlsvr... ibmi)

'connections' => [ 'ibmi' => ['driver => '', 'username' => ''....], 'mysql => [], 'pgsql' => [], ...]

is injected in the config array for each connection as the name (I mean what you did manually in the config file). But since my package is an extension to default connections I have to do the same thing.

And if you look in the official documentation you will never see the 'name' attribute. See here https://github.com/laravel/laravel/blob/master/config/database.php

So I did the fix in the last release. After you update then remove the 'name' entry in your config file.

Tell me if that's OK.

sarahkemp commented 7 years ago

The newest version (5.4.4.2) does fix this issue, thank you.

I looked through the readme and composer file for information about what versions of Laravel this was compatible with and didn't find anything, so I hoped it would work with 5.2* still. I plan to update as soon as I can, but it does seem compatible at the moment.

Thank you for the fix!

*Edit: my mistake, I'm on 5.3, not 5.2