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

Setting default db adapter in installation as PDO MySQL #6274

Merged
merged 1 commit into from
Nov 3, 2016
Merged

Setting default db adapter in installation as PDO MySQL #6274

merged 1 commit into from
Nov 3, 2016

Conversation

SpiritLevel
Copy link
Contributor

Following up on #6230.

I also added "Re-check requirements" button to CMS Admin Account section of config-form.html.

This solution works but is not all that satisfactory to me. There is repetition and I don't know how the error is best handled. More generally, the code between lines 140 and 188 seems awkward.

…s fail safe.

Added back missed assignment of $_REQUEST['db']['type']
@tractorcow tractorcow merged commit 857caa8 into silverstripe:master Nov 3, 2016
@tractorcow
Copy link
Contributor

thanks @SpiritLevel great patch

@SpiritLevel SpiritLevel deleted the PDO-as-default-in-webinstaller branch November 3, 2016 05:33
@SpiritLevel
Copy link
Contributor Author

Good :) I think I might have a go at a refactoring of what's in framework/src/Dev/Install...

@micmania1
Copy link
Contributor

micmania1 commented Oct 29, 2018

@SpiritLevel @tractorcow can any of you remember why this line wasn't included here? https://github.com/silverstripe/silverstripe-framework/pull/6230/files#diff-f8f7d711330a3259d575d72fc7e5a5cfR155

This has caused problems for us as local environments were defaulted to PDO (as per .env) but UAT ignored .env and was using the framework default of MySQLi. We found a bug in MySQLi connector which meant we could only re-produce the bug on UAT and it was difficult to track this down as the cause.

At the moment framework defaults to MySQLi and the installer defaults to PDO.

cc @stojg @chillu

note: as an aside, there is a PR for the problem with MySQLi - i'm purely talking about the differences in defaults here.

@micmania1
Copy link
Contributor

I've opened an issue here: #8540

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

Successfully merging this pull request may close these issues.

None yet

4 participants