-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Draft for an event with all information after schema change was perfomed #11409
base: 3.2.x
Are you sure you want to change the base?
Conversation
$eventManager = $this->em->getEventManager(); | ||
$eventManager->dispatchEvent( | ||
ToolEvents::postSchemaChanged, | ||
new SchemaChangedEventArgs($this->em, $schema, new Schema(), $createSchemaSql), |
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.
thank you for looking into this. so the idea would be that e.g. symfony messsenger could listen to this, inspect the changes and fire its own sql statements against the connection if it sees that something was done which it wants to alter?
is this event also useful when doctrine migrations are used rather than database updated on the fly?
with dbal 3, symfony was adding extra sql in https://github.com/symfony/symfony/blob/73f8713e143c64dc5f045dcdee7d5e893cccfcd8/src/Symfony/Bridge/Doctrine/SchemaListener/MessengerTransportDoctrineSchemaListener.php#L76C1-L76C38 (and that sql afaik can not be represented in the doctrine schema models.
that sql got saved to the migration file, which was quite nice to review it.
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.
Yes, something like:
public function onPostSchemaChanged($event)
{
if (!$this->hasMessengerTable($event)) {
$event->getEntityManage()->getConnection()->executeStatement($this->getMessengerTableSql());
}
}
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.
and how can this be integrated with the doctrine migrations? could the listener also add sql into a migration?
Cc @wmouwen |
Just to make it clear, I would greenlight this approach, but I need someone else to take over and finish this:
|
For now, that test only works with DBAL 3, but hopefully that's only because of the test mocking system rather than the code.
/cc @dbu