Returning file *path* as ->getDatabase() for SQLite
#4982
Conversation
I'm suggesting to return the file path instead of the database name for SQLite. Right now, when loading fixtures, the CLI prompt reads: > Careful, database "" will be purged. Do you want to continue? (yes/no) This is due to `LoadDataFixturesDoctrineCommand::execute()` at https://github.com/doctrine/DoctrineFixturesBundle/blob/3.4.x/Command/LoadDataFixturesDoctrineCommand.php#L106 I don't know where else `Connection::getDatabase()` gets called, but I'm figuring that returning the path is always a good idea :-)
|
SQLite doesn't support the concept of a database. Instead, we should consider throwing an exception there. See #4963 for more details. |
|
My first question is: Since the fixtures command did work until recently, was there a change here in DBAL or in the fixtures bundle? I couldn't figure anything out in GitHub's history :-( For the larger topic: The main question I'm seeing is: If somebody is calling |
The relevant change in the DBAL was released in
This behavior wouldn't be portable. Each method has its semantics, and a database has a certain meaning in the DBAL. The SQLite platform doesn't support these semantics, so returning something from this method which is not a database may help solve some cases but become an unpleasant surprise in others. If a tool wants to display an informational message, it should do so based on the connection configuration. |
Since DBAL 3.0 (see doctrine/migrations#1028), the message currently reads for SQLite: > Careful, database "" will be purged. Do you want to continue? (yes/no) Neither @Ocramius (see doctrine/dbal#3606 (comment)) nor me (see doctrine/dbal#4982) succeeded in convincing @morozov to continue returning the file path ;-) So it looks like the fix needs to be done here... Suggestion: What about adding the platform too? So the message would be: > Careful, MySQL database "foo" will be purged. Do you want to continue? (yes/no) `doctrine/migrations` is currently broken too: > WARNING! You are about to execute a migration in database "<unnamed>" that could result in schema changes and data loss. Are you sure you wish to continue? (yes/no) [yes]: So when done here, I'm going to submit the same there (i.e. follow up of doctrine/migrations#1028)
|
OK, thanks, so I'm closing here in favor of doctrine/DoctrineFixturesBundle#359 :-) |
Summary
I'm suggesting to return the file path as "database name" for SQLite (instead of nothing)
Right now, when loading fixtures, the CLI prompt reads:
This is due to
LoadDataFixturesDoctrineCommand::execute()at https://github.com/doctrine/DoctrineFixturesBundle/blob/3.4.x/Command/LoadDataFixturesDoctrineCommand.php#L106I don't know where else
Connection::getDatabase()gets called, but I'm figuring that returning the path is always a good idea :-)The query syntax is taken from https://stackoverflow.com/a/44279467/1668200