-
-
Notifications
You must be signed in to change notification settings - Fork 215
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
Added condition to check that we have an active connection #474
base: 3.3.x
Are you sure you want to change the base?
Added condition to check that we have an active connection #474
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a test that reproduces the problem you'd like to fix?
} | ||
|
||
public function collect(Request $request, Response $response, \Throwable $exception = null) | ||
public function collect(Request $request, Response $response, ?Throwable $exception = null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please note that running phpcbf is adding return type.
It's hard to reproduce this scenario in CI, as we need to boot entire application (without configured dbal connection) and I haven't found any "functional tests" for this bundle. |
@derrabus any chance that you'll be able to take a look at this? |
@derrabus Whats problem? |
I'm sorry that nobody has picked up this PR after my initial triage. I still don't fully understand why problem is solved here. The collector needs a valid database connection. And without one, this whole bundle doesn't make much sense. |
^^^ and the idea was: avoid collecting migrations if connection is not configured. |
and PR that was trying to fix similar issue: #428 just wrapped code with try/catch which does not prevent of trying to make a connection |
Fixes #473