Make multiple db connections work #18
Conversation
- user can config the db connection
make multiple db's working
update readme for new config option
|
Hello Sascha, Thanks for the pull request! Can you make it that we have a fallback (default) database connection, so that setting a database isn't required? |
|
Hej @vblinden : Sry, i missed that, here it is ;) |
- making the readme more clear
|
Wouldn't setting the connection in the constructor break when using static methods? From what I remember you need to set the
I don't have a project here right now to test if this is an issue or not. But if it is, perhaps we need another way to set the connection. |
When setting the |
| * The database connection to be used | ||
| * This will use the db connection from config/database.php | ||
| */ | ||
| 'db_connection' => env('DB_CONNECTION'), |
There was a problem hiding this comment.
This is a potential breaking change and/or it can lead to strange behaviour. Someone might use a different environment variable or has a fixed value for config('database.default'). If we default this to null and retrieve the value using config('laravel-fulltext.db_connection') ?? config('database.default') we make sure we use the same value Laravel uses now, while still allowing a custom connection.
| public function __construct() | ||
| { | ||
| parent::__construct(); | ||
| $this->connection = Config::get('laravel-fulltext.db_connection', config('database.default')); |
There was a problem hiding this comment.
This project uses the config helper instead of the facade. I prefer to keep it consistent, certainly in one line.
| @@ -12,7 +12,7 @@ class CreateLaravelFulltextTable extends Migration | |||
| */ | |||
| public function up() | |||
| { | |||
| Schema::create('laravel_fulltext', function (Blueprint $table) { | |||
| Schema::connection(config('laravel-fulltext.db_connection'))->create('laravel_fulltext', function (Blueprint $table) { | |||
There was a problem hiding this comment.
Don't forget to add the default/fallback here!
Description
When having multiple db connection, one can choose which one to use
Motivation and context
Having multiple db connections and needed to have fulltext-index NOT in the default one, this lead me to this ;)
How has this been tested?
Build into our system and let it run on a "not common" connection -> work
Put it into a default laravel and let it run unchanged -> works a before
Screenshots (if appropriate)
Types of changes
What types of changes does your code introduce? Put an
xin all the boxes that apply:Checklist:
Go over all the following points, and put an
xin all the boxes that apply.Please, please, please, don't send your pull request until all of the boxes are ticked. Once your pull request is created, it will trigger a build on our continuous integration server to make sure your tests and code style pass.
If you're unsure about any of these, don't hesitate to ask. We're here to help!