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

refactor: order host, port, driver the same both places in Drupal 10 settings file #5570

Merged
merged 3 commits into from Nov 27, 2023

Conversation

gitressa
Copy link
Contributor

The Issue

It is not a big problem, but the order of host, port, driver is switched, causing a slight "hmm" when you scan the values.

$host = "db";
$port = 3306;
$driver = "mysql";

$databases['default']['default']['database'] = "db";
$databases['default']['default']['username'] = "db";
$databases['default']['default']['password'] = "db";
$databases['default']['default']['host'] = $host;
$databases['default']['default']['driver'] = $driver;
$databases['default']['default']['port'] = $port;

How This PR Solves The Issue

Makes the order identical to the one in the default settings.php file:

 * $databases['default']['default'] = [
 *   'database' => 'databasename',
 *   'username' => 'sqlusername',
 *   'password' => 'sqlpassword',
 *   'host' => 'localhost',
 *   'port' => '3306',
 *   'driver' => 'mysql',
[...]

https://github.com/drupal/drupal/blob/10.1.x/sites/default/default.settings.php#L80-L85

Manual Testing Instructions

Automated Testing Overview

Related Issue Link(s)

Release/Deployment Notes

@gitressa gitressa requested a review from a team as a code owner November 25, 2023 19:19
Copy link

github-actions bot commented Nov 25, 2023

Copy link
Member

@rfay rfay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why it makes a difference, but fine with me :)

I assume that the d8 and d9 have the same difference?

@gitressa
Copy link
Contributor Author

gitressa commented Nov 25, 2023

It's a tiny change, but now they are aligned in their order :)

@rfay
Copy link
Member

rfay commented Nov 25, 2023

Order is good, right?

@gitressa
Copy link
Contributor Author

Yes, indeed :)

The reason I noticed it, was because I wanted to run MySQLTuner, and needed the host/port info. It took me a while to finally just try replacing 127.0.0.1 with db, and then I got a reply:

$ ddev ssh
ddev:$ git clone --depth 1 -b master https://github.com/major/MySQLTuner-perl
ddev:$ cd MySQLTuner-perl/
ddev:/MySQLTuner-perl$ perl mysqltuner.pl --host db
 >>  MySQLTuner 2.3.2
   * Jean-Marie Renouard <jmrenouard@gmail.com>
   * Major Hayden <major@mhtx.net>
 >>  Bug reports, feature requests, and downloads at http://mysqltuner.pl/
 >>  Run with '--help' for additional options and output filtering

[--] Skipped version check for MySQLTuner script
[!!] The --forcemem option is required for remote connections
[!!] Assuming RAM memory is 1Gb for simplify remote connection usage
[!!] The --forceswap option is required for remote connections
[!!] Assuming Swap size is 1Gb for simplify remote connection usage
[--] Performing tests on db:3306
[--] Using mysql to check login
[...]

@rfay
Copy link
Member

rfay commented Nov 25, 2023

ddev describe tells you all the things,

and https://ddev.readthedocs.io/en/latest/users/usage/database-management/

@gitressa
Copy link
Contributor Author

Thanks for the tips, reading the docs closer, this would have gotten me on track:

The web and db containers are each ready with MySQL/PostgreSQL clients, so you can ddev ssh or ddev ssh -s db and use mysql or psql.

@rfay rfay merged commit 3525414 into ddev:master Nov 27, 2023
18 checks passed
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

2 participants