Added condition to check that we have an active connection
Fixes #473
Can you add a test that reproduces the problem you'd like to fix?
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.
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.
It's been awhile, let me remember.
Collector still requires an configure connection. When starting new project and database is not yet configured, MigrationsCollector is trying to connect to database and because of PDO timeout, page feels are hanged.
IIRC initial problem is/was with clean project, when database is not yet configured.
So when I install this bundle it's automatically, it's automatically starts collecting information about executed migrations
and since database is not yet configured it's trying to connect to a database (hosts does not exist) and it's taking 30s (default timeout) before it's throwing an exception.
and it's not very obvious that something is wrong and developers start digging into a problem why clean project is taking 30s to load.
At least this is what I remember.
^^^ and the idea was: avoid collecting migrations if connection is not configured.
and PR that was trying to fix similar issue: https://github.com/doctrine/DoctrineMigrationsBundle/pull/428 just wrapped code with try/catch which does not prevent of trying to make a connection