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

Allow running dbal:run-sql using arbitrary connection #3950

Closed
wants to merge 4 commits into from
Closed

Allow running dbal:run-sql using arbitrary connection #3950

wants to merge 4 commits into from

Conversation

delolmo
Copy link

@delolmo delolmo commented Apr 14, 2020

Q A
Type feature
BC Break no

Summary

This slight modification allows running the dbal:run-sql command using an arbitrary database connection, which is specially useful in CLI applications that use multiple database connections. Backwards compatibility is maintained by using the default 'db' value for the input option.

The command now includes a --connection option, which defaults to 'db'. If specified, a ConnectionHelper is searched for that matches the provided connection name.

Antonio del Olmo and others added 4 commits April 14, 2020 16:59
This slight modification allows running the `dbal:run-sql` command using an arbitrary database connection, which is specially useful in CLI applications that use multiple database connections. Backwards compatibility is maintained by using the default 'db' value for the input option.
@SenseException
Copy link
Member

If specified, a ConnectionHelper is searched for that matches the provided connection name.

How do you add a new ConnectionHelper with a new connection in your use case? Can you please provide example code? This might be relevant because adding a new helper for another connection should become part of the documentation in scope of this PR if this gets approved.

@morozov
Copy link
Member

morozov commented Apr 17, 2020

There's another similar pull request #3956.

@delolmo
Copy link
Author

delolmo commented Apr 17, 2020

How do you add a new ConnectionHelper with a new connection in your use case? Can you please provide example code? This might be relevant because adding a new helper for another connection should become part of the documentation in scope of this PR if this gets approved.

When defining a generic \Symfony\Component\Console\Application, a HelperSet may be passed through the setHelperSet method. In doctrine/migrations, e.g., a cli-config.php script is created returning a specific HelperSet which can be modified to include several ConnectionHelper's:

<?php

require 'vendor/autoload.php';

use Doctrine\DBAL\DriverManager;
use Doctrine\DBAL\Tools\Console\Helper\ConnectionHelper;
use Symfony\Component\Console\Helper\HelperSet;

$dbParams = include 'migrations-db.php';

$connection = DriverManager::getConnection($dbParams);

return new HelperSet([
    'db' => new ConnectionHelper($connection),
    'db.read' => new ConnectionHelper($connection), // for example
    'db.write' => new ConnectionHelper($connection), // for example
]);

For documentation purposes, I was thinking about the following example:

<?php

use Doctrine\DBAL\DriverManager;
use Doctrine\DBAL\Tools\Console\Helper\ConnectionHelper;
use Doctrine\DBAL\Tools\Console\Command\RunSqlCommand;
use Symfony\Component\Console\Application;
use Symfony\Component\Console\Helper\HelperSet;

$dbParams = [...]; // A multidimensional array with connection parameters

$cli = new Application('app name', 'app version');

$cli->setHelperSet(new HelperSet([
    'db.read' => new ConnectionHelper(
        DriverManager::getConnection($dbParams['db.read'])
    ),
    'db.write' => new ConnectionHelper(
        DriverManager::getConnection(($dbParams['db.write'])
    ),
    'pgsql' => new ConnectionHelper(
        DriverManager::getConnection($dbParams['db.pgsql'])
    ),
    'mysql' => new ConnectionHelper(
        DriverManager::getConnection(($dbParams['db.mysql'])
    ),
    // and so on...
]));

$cli->addCommand(new RunSqlCommand());

// Continue configuring your cli app, add commands, etc.

Regarding #3956, IMO its too much code to solve a very specific use case. Maybe, a new ConnectionProviderHelper could be created, but I still think its unnecessary.

I would be happy to add this same modification to the ReservedWordCommand, if we are in agreement.

@delolmo
Copy link
Author

delolmo commented Apr 17, 2020

BTW I think I mentioned doctrine/migrations, but doctrine/dbal and doctrine/orm use a similar approach to define database connections.

doctrine/dbal expects a HelperSet to be returned by the cli-config.php file, while doctrine/orm uses a createHelperSet static method.

@dmaicher
Copy link
Contributor

Either we go with this approach (which would be backwards compatible without deprecations) or we move forward with #3956

@morozov what do you think?

@morozov
Copy link
Member

morozov commented May 11, 2020

@dmaicher, I'd proceed with #3956.

@morozov
Copy link
Member

morozov commented Jun 13, 2020

Closing in favor of #3956.

@morozov morozov closed this Jun 13, 2020
@morozov morozov self-assigned this Jun 13, 2020
@delolmo delolmo deleted the patch-1 branch June 17, 2020 15:30
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants