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

[3.0-alpha] Seamless 2.0 -> 3.0 upgrade, keeping migration versions data #967

Closed
pamil opened this issue Apr 22, 2020 · 6 comments
Closed
Labels
Milestone

Comments

@pamil
Copy link

pamil commented Apr 22, 2020

Is there a recommended way to upgrade an existing application to use Migrations 3.0 and not lose migration versions data?

Ideally, after updating the package versions and adjusting the configuration, Migrations 3.0 would know which migrations have already been applied by Migrations 2.0.

My ideas:

  1. An extension point to transform previous migration versions to the new format. For example, I could define that 20170101235959 is now Sylius\CoreBundle\Migrations\Version20170101235959 and it would execute before actually running the migrations.

  2. More of a meta-solution. Does it make sense to make a migration that migrates that data? Migration that inserts rows into the new migrations versions table based on the old one.

@goetas
Copy link
Member

goetas commented Apr 22, 2020

That should be already automatically handled by

private function updateMigratedVersionsFromV1orV2toV3() : void

if is not the case, there might be a bug. Did you already try it?

@emodric
Copy link
Contributor

emodric commented Apr 22, 2020

I had no problems with keeping migrations data. I did have to run doctrine:migrations:sync-metadata-storage to upgrade to the new storage format, though.

On a related note, @goetas would it be possible to run doctrine:migrations:sync-metadata-storage command automatically the first time doctrine:migrations:migrate command is ran after upgrading to v3?

@goetas
Copy link
Member

goetas commented Apr 22, 2020

On a related note, @goetas would it be possible to run doctrine:migrations:sync-metadata-storage command automatically the first time doctrine:migrations:migrate command is ran after upgrading to v3?

That should have been already the case.
Will double check it.

@pamil
Copy link
Author

pamil commented Apr 22, 2020

I'll try whether I can reproduce it.

@pamil
Copy link
Author

pamil commented Apr 22, 2020

It was mostly my fault, I deleted table_name configuration option instead of setting its equivalent in the new configuration. After configuring migrations to use the same table, migration works, but only after running sync-metadata-storage:

Screenshot 2020-04-22 at 12 17 15

@goetas
Copy link
Member

goetas commented Apr 30, 2020

Will be fixed in #973

@goetas goetas added this to the 3.0.0 milestone Apr 30, 2020
@goetas goetas added the Bug label Apr 30, 2020
@goetas goetas closed this as completed May 1, 2020
pamil added a commit to Sylius/Sylius that referenced this issue Aug 24, 2020
This PR was merged into the 1.8-dev branch.

Discussion
----------

### Customisations
 - [x] Decorated migration factory to support migrations implementing `ContainerAwareInterface` - done in CoreBundle
 - [x] Added topology-based version comparator in CoreBundle to achieve the following order:
   - Sylius/Sylius
   - Plugins (Sylius/PluginSkeleton: Sylius/InvoicingPlugin, Sylius/RefundPlugin)
   - Sylius/Plus 
   - End-user application (Sylius/Sylius-Standard)

### Concerns
 - [x] **Q:** Currently, migrations are doubled in `app/migrations` and `src/Sylius/Bundle/CoreBundle/Migrations` - could we safely delete the ones in `app/migrations`?

    **A:** It should be possible to delete them in the next minor release - while migrating migrations database, it correctly assigns FQCN of the migration by its timestamp. However, we need to figure out how to upgrade existing apps, especially after we recommended copying migrations to the end-user's project before.

 - [x] **Q:** All the migrations exist in CoreBundle, even though some of them contain SQL queries related to AdminApiBundle - what's the best way to tackle that (one might not use AdminApiBundle but only CoreBundle)? Should we split them into two bundles? If so, how to do it and allow for a seamless upgrade?

    **A:** They can be safely extracted to AdminApiBundle as long as there are no migrations mixing both CoreBundle and AdminApiBundle. In that case, I'm not sure what's the best solution - probably skipping some of them if AdminApiBundle is not installed.

### Up next for upgrade to migrations 3.0-alpha
 - [ ] Sylius-Standard
 - [ ] PluginSkeleton
 - [x] InvoicingPlugin - Sylius/InvoicingPlugin#175
 - [x] RefundPlugin - Sylius/RefundPlugin#205
 - [x] Plus (requiring InvoicingPlugin and RefundPugin) - https://github.com/Sylius/Plus/pull/575

### TODO

1. [x] Extract related utilities (comparator and factory) to a standalone package (SyliusLabs/DoctrineMigrationsExtraBundle?) with a following configuration schema:
   ```yaml
   sylius_labs_doctrine_migrations_extra:
      migrations:
         "Sylius\InvoicingPlugin\Migrations": ["Sylius\Bundle\CoreBundle\Migrations"]
   ```
   Tests need to be added for both those services, configuration and DI extension.

2. [x] Use bundle notation for specifying migrations instead of prepending `doctrine_migrations` with relative paths, see doctrine/DoctrineMigrationsBundle#318.

3. [x] Decouple CoreBundle migrations from AdminApiBundle (either conditionally add them or move somehow to AdminApiBundle, while maintaining BC - after running all migrations on `master` branch, checking in this branch should result in no additional migrations)

4. [ ] Spread those changes to all the repositories mentioned above

### References
 - https://www.goetas.com/blog/multi-namespace-migrations-with-doctrinemigrations-30/
 - https://www.goetas.com/blog/released-doctrinemigrations-30-alpha/
 - doctrine/migrations#967
 - doctrine/DoctrineMigrationsBundle#314

Commits
-------

0bf2c7e Move existing migrations to Sylius\Bundle\CoreBundle\Migrations
66ef7dd Allow injecting container to the migrations & implement a topology-based sorter
aca4fd2 Split migrations between CoreBundle & AdminApiBundle
0d22ea0 Improve SyliusCoreExtension prepend logic
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants