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

Check if DBAL connection is available from .env configuration #1547

Merged
merged 4 commits into from Mar 23, 2020

Conversation

@bytehead
Copy link
Member

bytehead commented Mar 22, 2020

Q A
Fixed issues n/a
Docs PR or issue contao/docs#307

During my work for contao/docs#300 I stumbled over this.
The installation tool asks for the database connection, even if there is a connection set up via the DATABASE_URL variable in .env.

This happens if you have symfony/website-skeleton ^3.4 as a base application and Contao 4.4.* installed on top as a bundle.

Edit:
It works in Contao 4.9, see a2c25b4.

@bytehead bytehead added the defect label Mar 22, 2020
@bytehead bytehead added this to the 4.4 milestone Mar 22, 2020
@bytehead bytehead requested a review from contao/reviewers Mar 22, 2020
@bytehead bytehead self-assigned this Mar 22, 2020
@bytehead bytehead changed the title Check if connection is available from .env configuration Check if DBAL connection is available from .env configuration Mar 22, 2020
bytehead added 3 commits Mar 22, 2020
@leofeyer leofeyer merged commit 9837273 into contao:4.4 Mar 23, 2020
8 of 9 checks passed
8 of 9 checks passed
Coverage
Details
Coding Style
Details
PHP 5.6
Details
PHP 7.1
Details
PHP 7.2
Details
PHP 7.3
Details
Prefer Lowest
Details
Bundles
Details
Windows Windows
Details
@leofeyer

This comment has been minimized.

Copy link
Member

leofeyer commented Mar 23, 2020

Thank you @bytehead.

@bytehead bytehead deleted the bytehead:fix/installation-controller-with-dotenv branch Mar 23, 2020
// Return if there is a working database connection already
try {
$this->connection->connect();
$this->connection->query('SHOW TABLES');

return true;
} catch (\Exception $e) {
}

Comment on lines +139 to +147

This comment has been minimized.

Copy link
@aschempp

aschempp Mar 25, 2020

Contributor

wouldn't this break if $this->connection is null (checked on the next line)?

This comment has been minimized.

Copy link
@leofeyer

leofeyer Mar 25, 2020

Member

The PR is an exact backport of a2c25b4, which we have added in Contao 4.6. 🤷‍♂

This comment has been minimized.

Copy link
@bytehead

bytehead Mar 25, 2020

Author Member

Why should it break? It's wrapped in a try/catch block?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.