8000 Fix migrations preup with table prefix and migration performance by rohitpavaskar · Pull Request #11371 · mautic/mautic · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix migrations preup with table prefix and migration performance #11371

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Aug 29, 2022
Merged

Fix migrations preup with table prefix and migration performance #11371

merged 8 commits into from
Aug 29, 2022

Conversation

rohitpavaskar
Copy link
Contributor
@rohitpavaskar rohitpavaskar commented Aug 2, 2022 •
edited by gitpod-io bot
Loading
Q A
Bug fix? (use the a.b branch) [Y]
New feature/enhancement? (use the a.x branch) [N]
Deprecations? [N]
BC breaks? (use the c.x branch) [N]
Automated tests included? [N]
Related user documentation PR URL mautic/mautic-documentation#...
Related developer documentation PR URL mautic/developer-documentation#...
Issue(s) addressed Fixes #...

Description:

This PR fixes issue with correct preUp migration conditions with and without prefix.
Also optimise query generation

Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. Run the migrations on old version of Mautic and check migrations are successful

@cla-bot cla-bot bot added the cla-signed The PR contributors have signed the contributors agreement label Aug 2, 2022
escopecz
escopecz previously approved these changes Aug 2, 2022
View reviewed changes
Copy link
Member
@escopecz escopecz left a comment

Choose a reason for hiding this comment

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

Thanks Rohit! 👍

@escopecz escopecz added pending-test-confirmation PR's that require one test before they can be merged db-migrations labels Aug 2, 2022
@RCheesley
Copy link
Member

Thanks @rohitp19 - would you be able to rebase this on the 4.4 branch so we could get it into the next bug fix release?

@RCheesley
Copy link
Member

Presumably to test this we need an old (maybe Mautic 3?) version of Mautic with a database prefix in use to test the migrations are working correctly, is that a good understanding of the testing required?

kuzmany
kuzmany previously approved these changes Aug 12, 2022
View reviewed changes
Copy link
Member
@kuzmany kuzmany left a comment

Choose a reason for hiding this comment

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

Code looks good.
I've run all migrations. Does not see issue
We can go 👍

@kuzmany kuzmany added ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged and removed pending-test-confirmation PR's that require one test before they can be merged labels Aug 14, 2022
@escopecz escopecz changed the base branch from 4.x to 4.4 August 29, 2022 13:13
@escopecz escopecz changed the base branch from 4.4 to 4.x August 29, 2022 13:14
@escopecz escopecz dismissed stale reviews from kuzmany and themself August 29, 2022 13:14

The base branch was changed.

@escopecz escopecz self-assigned this Aug 29, 2022
@escopecz
Copy link
Member

I checked how hard it would be to rebase this on 4.4 which is the bug fix branch and there were conflicts in 86 files. Gonna merge it in 4.x and it will take longer to get to production. @rohitpavaskar please use the right branch in the future so it can be released sooner.

@escopecz escopecz added this to the 4.5.0 milestone Aug 29, 2022
@escopecz escopecz merged commit 36260c5 into mautic:4.x Aug 29, 2022
@escopecz
Copy link
Member
escopecz commented Sep 5, 2022

There will be no more feature releases for M4. I'll merge this into M5 (the 5.x branch)

@escopecz escopecz modified the milestones: 4.5.0, 5.0-alpha Sep 5, 2022
@escopecz escopecz added the bug Issues or PR's relating to bugs label Sep 5, 2022
daniellord32 pushed a commit to daniellord32/mautic that referenced this pull request Mar 20, 2023
@rohitpavaskar @rohitp19
@escopecz @daniellord32
Fix migrations preup with table prefix and migration performance (mau…
a8236b2 
…tic#11371)

* Fixed migration prefix index issue

* Fix Version20191219155630 migration to 100% cover perUp condition

* Fix Version20191017140848 migration to 100% cover perUp condition

* Fix Version20200227110431 migration to 100% cover preUp condition to check if index is present

* Fix Version20200805185714 migration to 100% cover preUp condition

* Fixed isse with prefix and innodb_strict_mode

* Fixed CSFixer

Co-authored-by: Rohit Pavaskar <rohit.pavaskar@acquia.com>
Co-authored-by: John Linhart <admin@escope.cz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kuzmany kuzmany kuzmany left review comments

@escopecz escopecz escopecz left review comments

Assignees

@escopecz escopecz

Labels
bug Issues or PR's relating to bugs cla-signed The PR contributors have signed the contributors agreement db-migrations ready-to-commit PR's with 2 successful tests, 1 approval, automated tests and docs and is ready to be merged
Projects
None yet
Milestone
5.0-alpha
Development

Successfully merging this pull request may close these issues.
5 participants
@rohitpavaskar @RCheesley @escopecz @kuzmany @rohitp19
0