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

[5.8] Adds the ability to set the reconnector created by a `DatabaseManager`. #27845

Merged
merged 2 commits into from Mar 13, 2019

Conversation

Projects
None yet
3 participants
@iainconnor
Copy link

iainconnor commented Mar 10, 2019

Implements laravel/ideas#1557.

Our current use-case for wanting to customize the reconnector is wanting to add a "maximum retries" setting. When experiencing some edge-case database issues, they were exacerbated by the default reconnector "infinitely" trying to reconnect (infinitely in quotes because eventually something times out, but still has the tendency to turn a minor blip into a feedback loop).

@GrahamCampbell GrahamCampbell changed the title #1557 Adds the ability to set the reconnector created by a `DatabaseManager`. [5.8] Adds the ability to set the reconnector created by a `DatabaseManager`. Mar 11, 2019

@taylorotwell

This comment has been minimized.

Copy link
Member

taylorotwell commented Mar 12, 2019

So, in a service provider, what prevents you from doing something like: DB::connection('foo')->setReconnector($reconnector)... that method is already public? Maybe even something like this in a boot method of a service provider so it's only run when the DB manager is actually resolved (it's bound as "db" in the container):

$this->app->resolved('db', function ($db) {
    $db->connection('foo')->setReconnector(fn);
});
@iainconnor

This comment has been minimized.

Copy link
Author

iainconnor commented Mar 12, 2019

I have multiple connections, so I'd need to iterate through (and configure) all of them in this block of code, when any given request may not actually utilize (and thus not have to configure) those connections.

Yes, the overhead of the configuration is (relatively) minor, but still unnecessary until I actually need to perform a query against the Db.

@taylorotwell taylorotwell merged commit 8fd4a19 into laravel:5.8 Mar 13, 2019

2 checks passed

continuous-integration/styleci/pr The analysis has passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.