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

BUGFIX: Persist doctrine MetadataStorage changes immediately #2333

Merged
merged 1 commit into from Jan 4, 2021

Conversation

bwaidelich
Copy link
Member

@bwaidelich bwaidelich commented Dec 9, 2020

This basically adds a EntityManager::flush() call after MetadataStorage::ensureInitialized()
invocations in order to apply any metadata migrations immediately.

Without this a ./flow doctrine:migrate call might leave the metadata storage in a half-migrated
state if it leads to an exception.

Related: #2244
Related: #2357

Copy link
Member

@kitsunet kitsunet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sooo weird that it's necessary but well, if that's what is needed....

Copy link
Member

@kdambekalns kdambekalns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it helps… 🤔

@kdambekalns
Copy link
Member

kdambekalns commented Dec 9, 2020

I am really at a loss here… Why? I tried PHP errors in migrations, invalid SQL in migrations, re-applying already executed migrations – and did not need that (again).

But, whatever, if it solves if, do it. 🤷‍♂️

This doesn't do anything like that, too: https://github.com/doctrine/migrations/blob/3.0.x/lib/Doctrine/Migrations/Tools/Console/Command/MigrateCommand.php

@bwaidelich
Copy link
Member Author

@kdambekalns Mh, you're right (as usual).. And I can't reproduce this any longer..
What I tried:

UPDATE flow_doctrine_migrationstatus SET version = RIGHT(version, 14);
ALTER TABLE flow_doctrine_migrationstatus DROP executed_at, DROP execution_time;

(to revert the metadata table to its old state)

DELETE FROM flow_doctrine_migrationstatus WHERE version = '20200908155620';

to remove the migration that was missing in my case.

Previously that led to a swallowed error and the half-migrated table.
Now (without this PR applied) I get an exception

An exception occurred while executing 'ALTER TABLE neos_flow_resourcemanagement_persistentresource DROP md5':

SQLSTATE[42000]: Syntax error or access violation: 1091 Can't DROP COLUMN `md5`; check that it exists

(as expected)

and the table is fully migrated..

Probably I was in some weird state when I tried it last.

I'll close this one for now and hope that this heisenbug never returns :)

@bwaidelich bwaidelich closed this Dec 10, 2020
@kdambekalns
Copy link
Member

I'll close this one for now and hope that this heisenbug never returns :)

🤞

@kdambekalns
Copy link
Member

Wait, we never merged this…

@kdambekalns kdambekalns reopened this Dec 22, 2020
@albe
Copy link
Member

albe commented Dec 22, 2020

I thought that was because the issue that this seemed to solve was no longer reproducible without this change?

Now (without this PR applied) I get an exception
(as expected)
and the table is fully migrated..

@kdambekalns
Copy link
Member

@albe Yes, but see #2357

@albe
Copy link
Member

albe commented Dec 22, 2020

🤔 well, yeah. If this fixes those cases...

PS: Would like to create bugfix releases tomorrow or so, so should we merge this before?

@albe albe merged commit c81c889 into 7.0 Jan 4, 2021
@albe albe deleted the bugfix/2244-persist-metadatastorage-update-immediately branch January 4, 2021 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants