With Craft 3.4, many DbConfig.php
properties have been deprecated. While the docs state that Both approaches work, but setting DB_DSN instead is recommended in Craft 3.4 and later, a deprecation means that they will eventually go away.
I think this decision lacks empathy with the Craft DX. My suggestion would be:
DbConfig.php
propertiescraft
CLIconfig/db.php
settings that expect a DSNMy reasoning is:
Many people have been developing using Craft CMS for years, but have no idea what a dsn
is, and they are inherently more tricky to set up properly than separate attributes.
DSNs can also be different per-platform (in terms of case sensitivity, parameters, etc.), and having a Craft layer that configures these connection settings the right way regardless of platform is a good idea.
Consider a .env
file that looks like this:
DB_DRIVER=mysql
DB_SERVER=mariadb
DB_USER=project
DB_PASSWORD=project
DB_DATABASE=project
DB_SCHEMA=public
DB_TABLE_PREFIX=
DB_PORT=3306
vs one that looks like this:
DB_DSN=mysql:host=mariadb;port=3306;dbname=project
DB_USER=project
DB_PASSWORD=project
DB_SCHEMA=public
DB_TABLE_PREFIX=
Sure, I think people can figure out what these things in the DSN are. But with any DX change, I think it's important to keep in mind the _gain_ from said change. In this case, the vast majority if people who use Craft and have no idea what a DSN is will have no gain at all.
The small minority of people who do know what a DSN is, and find it useful in their work, were already able to use it directly anyway.
So I think reverting to the "opt-in" philosophy on the DSN is better than what Craft 3.4 has now, which is an "opt-out" philosophy for the DSN (and presumably eventual removal of anything but a DSN approach).
Moreover just removing the deprecation of these properties, but leaving them in the CLI is a half-solution that still leaves us in the "opt-out" methodology with little gain to the majority of developers.
Let's say I use the CLI to set things up, so I'll end up with a DB_DSN in my .env file, and my config/general.php will look like this:
<?php
/**
* Database Configuration
*
* All of your system's database connection settings go in here. You can see a
* list of the available settings in vendor/craftcms/cms/src/config/DbConfig.php.
*
* @see craft\config\DbConfig
*/
return [
'dsn' => getenv('DB_DSN'),
'user' => getenv('DB_USER'),
'password' => getenv('DB_PASSWORD'),
'schema' => getenv('DB_SCHEMA'),
'tablePrefix' => getenv('DB_TABLE_PREFIX'),
];
Great so I'm doing local development, everything is wonderful, it all works. I don't need to know what a DSN is even though I'm using it.
Then I push everything to production, where I need to manually create a new .env
file for this new environment. My host server is different, maybe my port is different, etc.
So then I use the existing local dev .env
as a guide, and now I need to edit this weird DB_DSN thing that I've never heard of and try to get it to work.
I can't even go back to just defining the old DB_DRIVER
, DB_SERVER
, etc. .env
variables, because for that to work, I'll have to edit the config/db.php
to use them (and look up what the keys were supposed to be).
This same process also gets repeated by every developer on my team who needs to learn what a DSN is to get this new project (created with composer create-project craftcms/craft
) working on their local environment.
I think this will create at least some support burden, and impact the Craft DX for little gain.
This, imo, does not solve the problem https://github.com/craftcms/cms/commit/04f31ac0f88632144603a7d8cf7689d249d351eb -- it's only a half-step.
The majority will still have to learn what a DSN is, and edit a dsn string, for no gain... because the Craft default project sets it up that way.
Without getting too into the weeds, I'll wholeheartedly agree that DSN should be entirely opt-in. I've been doing web dev a long time and know nothing about DSN... to be honest, I'm not sure how much I _want_ to know about DSN.
I'm quite happy using the tried & true pattern that I've been copy & pasting for 15 years...
Host Name
Username
Password
DB Name
I can't speak to all the other nuances, but thanks for at least un-deprecating the existing (fairly standard) connection parameters! 👍
The reason we switched is to be consistent with how read/write splitting is configured. It feels extremely awkward to me that slave connections would require using a DSN, whereas the master connection is configured using driver
, server
, port
, and database
settings.
Understood. And on paper, I totally get the desire for parity.
But the advanced Database Component management (that you linked to) will only ever be necessary on fewer than 1% of sites. For the folks getting their hands dirty with slave connections, they will have no problem messing about with DSN configurations. I doubt they'd find it weird that the "next level" DB configuration is handled a little differently.
I don't believe it's necessary to derail the other 99% of projects to suit this 1%. For everyone not affected by DSN or slave databases, they shouldn't even notice that anything has changed.
^ what @lindseydiloreto said
Also, think of poor Oli.
FWIW, @olivierbon might have a different experience, but I can't recall this ever coming up as an issue in support.
I wouldn't expect it to come up as an issue in support, because:
If a deprecator actually logged the deprecation, I would expect you'd see some support tickets.
If these settings are actually removed (which is the next step after deprecation), then I'd expect the support to be a factor.
As new projects created with 3.4 and Craft CLI eventually move from local dev to production, you may potentially see some support.
But I don't expect it to be a massive support issue. Rather, I'd ask: to what end?
Why do something that changes an existing feature from "opt-in" to "opt-out", with little gain (and some pain) for the vast majority?
As @lindseydiloreto said:
I don't believe it's necessary to derail the other 99% of projects to suit this 1%
I think 1% is being generous, really. I highly doubt that 1% of Craft sites, or even 1% of Craft developers, commonly implement read/write splitting for their Craft installs.
Totally agree @lindseydiloreto and @khalwat . I really wondered why it was implemented before I came here. A few weeks ago my latest Craft install crashed because of it. Luckily I remembered the release notes and know what DSN is so it took only a few seconds to fix it but my first thought was "there will be many support tickets for this in the future". It's quite surprising there are none.
Not really sure if that change is worth for the few people who use read/write splitting.
FWIW, @olivierbon might have a different experience, but I can't recall this ever coming up as an issue in support.
@angrybrad - Never had the issue.
I’ll consider switching back, but it’s a little weird to reconcile “this is a major DX issue” with the fact that this is the first time it has come up since 3.4 was released four months ago. ¯\_(ツ)_/¯
I didn't even notice the deprecations myself until a few weeks ago. I mentioned it to a number of Craft developers, and none of them had any idea the change had even been made.
So I'm not surprised that no one has contacted you, for the reasons I mentioned above. Once people start actually noticing the change, I think that may change.
But I may be wrong about that as well.
And then you'll have people like Robin who are impacted by the change, never contact you, but still are negatively affected for no real gain.
With any change, there is a cost that needs to be weighed against the gain from the change. I'm not seeing it here.
@Anubarak Good to hear from you again! Don't be a stranger around Discord!
I mean... it’s only deprecated (so still works), and there is nothing throwing a deprecation warning or error. Why _would_ you be seeing support requests about it?
My contention is that this will become a major issue in the future, if you force people to use the DSN pattern (for no obvious gain).
Also, what’s up @Anubarak! Miss ya bud!
I had only added the @deprecated
tags as a way of showing preference for $dsn
, but did not intend to actually remove those in Craft 4. And as of yesterday’s 3.4.21 release, the @deprecated
tags have been removed (04f31ac0f88632144603a7d8cf7689d249d351eb).
That’s all I need, thanks @brandonkelly! 🍺
I’m still not sure that DSN needs to be part of the Craft CLI default configuration, but that’s not a hill I’m gonna die on.
Right... but at least imo if the CLI is going to generate new projects with DB_DSN
in the .env
and the config/db.php
file with the same, both stripped of the settings Craft developers are familiar with... that it remains a problem.
In that people will end up with a confusing config setting they've never seen before, and it won't be beneficial to > 99% of them.
For people to start encountering this, they need to be:
.env
file editing is needed)...so I'd expect it to be on a delay. I don't think nearly enough time has passed to determine whether it's confusing to people or not.
Yeah okay, people can figure it out. But why?
Anyway, just my two cents. If you think there's a benefit that's worth it, then close up the issue.
I had only added the @deprecated tags as a way of showing preference for $dsn, but did not intend to actually remove those in Craft 4. And as of yesterday’s 3.4.21 release, the @deprecated tags have been removed (04f31ac).
Probably the docs could explain a reason for this preference, if we're asking people to switch something they are used to doing?
Unrelated, but... Pixel & Tonic may not be getting any support questions re: DSN yet (?), but I just got one! :)
I don't think it's fair to say "there have been no issues"... this exchange just took place on Discord. By the end of the exchange, the OP (a newcomer to Craft) even tags Brad Bell (ie: reaches out to support).
I think the person I'm doing support for on this issue summed it up:
Just addressed this for the next release. Going forward, if there is a DB_DSN
environment variable already defined (e.g. via .env
), then Craft will go with that; otherwise it will go with DB_DRIVER
, DB_SERVER
, DB_PORT
, and DB_DATABASE
.
After the next release is out, I’ll tag a new version of the craftcms/craft
starter project so the default .env
and config/db.php
files go with the split-up environment variables instead of DB_DSN
.
Thank you @brandonkelly !
Well done, @brandonkelly . I've kept silent, as I should, but have felt @khalwat Andrew has been a bit on the side of the angels here -- not to forget anyone else whose ears may be burning ;). Best to all...busy with inventing a pretty interesting cloud architecture for a covid-involved nonprofit, towards which Craft Bend, true ever to their ways, have been quite generous.
@lindseydiloreto fair, but I swear that was the first time it's happened. ;)
@narration-sd I'm happy to give you any plugin licenses you might want for your project as well.
@angrybrad There'd be more!
@khalwat very appreciated, Andrew -- I'd let you know, thank you
Craft 3.4.23 is out now with this change, as well as an update to the craftcms/craft
project.
Most helpful comment
Just addressed this for the next release. Going forward, if there is a
DB_DSN
environment variable already defined (e.g. via.env
), then Craft will go with that; otherwise it will go withDB_DRIVER
,DB_SERVER
,DB_PORT
, andDB_DATABASE
.After the next release is out, I’ll tag a new version of the
craftcms/craft
starter project so the default.env
andconfig/db.php
files go with the split-up environment variables instead ofDB_DSN
.