-
Notifications
You must be signed in to change notification settings - Fork 3
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
add dbal related code from DoctrineBundle #1
Conversation
some other stuff is missing (the doctrine/dbal dependency for instance) |
@stof yes this is far from completion 😄 Also need to add/move tests |
@dmaicher I think it might be easier to have one PR importing everything needed from the DoctrineBundle rather than splitting the import of the DI extension and other stuff (in particular given that it does not make sense to import the DI config for stuff that has not been imported yet) |
@stof the basics are working 😊 I tested it using a new skeleton project: /**
* @Route("/test", name="test")
*/
public function index(Connection $connection)
{
$connection->ping();
return new Response('<html><body>NICE</body></html>', 200, [
'Content-Type' => 'text/html',
]);
} |
@dmaicher what is the status of this work ? |
Thanks for your comments. I hope to get back to this within the next 1-2 weeks 😊 |
@stof so I propose to keep the registry inside this bundle. Can you have another look? Next steps would be:
|
src/Command/Proxy/RunSqlCommand.php
Outdated
/** | ||
* Execute a SQL query and output the results. | ||
*/ | ||
class RunSqlCommand extends DoctrineRunSqlCommand |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually since doctrine/dbal#3956 no need to extend the command anymore.
So we should require dbal 2.11 and inject an adapter from our registry to the ConnectionProvider
interface
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now I changed it so we support both < 2.11 and >= 2.11 without deprecations.
Or should we just require >= 2.11 already?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depends on the ETA for DBAL 2.11. If it is released soon enough for us to require it, I would go for that option.
Symfony 5.1 will drop support for DBAL <2.10 anyway and older DBAL version are not really maintained, so I'm fine with asking people to use an uptodate DBAL version when switching to our new bundle.
#SymfonyHackday
Status:
ConnectionRegistry
interface + PSR-11 implementationmovenot sure we still need itConnectionRegistry
andPsr11ConnectionRegistry
todoctrine/dbal
(see Add DBAL Connection Registry dbal#3892)2.0.x
done in the meantime