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

eliminate 2.3 to 3.x upgrade roadblocks #11131

Merged
merged 1 commit into from
Aug 9, 2017
Merged

eliminate 2.3 to 3.x upgrade roadblocks #11131

merged 1 commit into from
Aug 9, 2017

Conversation

hypeJunction
Copy link
Contributor

@hypeJunction hypeJunction commented Aug 8, 2017

chore(upgrade): eliminate 2.3 to 3.x upgrade roadblocks 

All synchronous upgrade scripts have been removed.

Scripts that were altering the database have been moved to phinx database
migrations.

Security settings defaults have been moved to async script to allow
admins access upgrade.php during upgrade process.

Async upgrade to convert database and table encoding to utf8 has been
added.

Database is now seeded before running upgrade tests on Travis.

Async upgrades now run during upgrade tests on Travis.

chore(tests): seed the database before running upgrade tests

  • Try a manual upgrade from 2.3, incl async upgrades

@@ -657,24 +657,18 @@ public static function upgrade() {
* @return bool
*/
public static function migrate() {
try {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am letting it throw here, otherwise it's hard to debug problems.

@@ -96,24 +96,6 @@ function set($name, $value) {
':name' => $name,
':value' => serialize($value),
];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sync upgrades will always run after database migrations, so this is not needed anymore

/**
* Removes site guid from legacy 2.x tables
*/
public function change() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I figure for most of these migrations downward migration will not be possible.

@hypeJunction
Copy link
Contributor Author

Maybe someone else can take a look. Things are working fine locally, so not sure what's up with Travis build.

use Phinx\Db\Adapter\MysqlAdapter;
use Phinx\Migration\AbstractMigration;

class ChangeTableEngineAndEncoding extends AbstractMigration {
Copy link
Member

Choose a reason for hiding this comment

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

I'm really torn about requiring this to upgrade to 3.0, though a controlled process like this is the way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It will be such a painful manual process otherwise

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One other option is to rename all old tables by adding a prefix or suffix, creating new schema and then migrating all values from old tables. It might be a bit cleaner

@hypeJunction
Copy link
Contributor Author

This is the best I can do here. If tests fail, then someone will need to take over.

@hypeJunction
Copy link
Contributor Author

@jdalsem Could you check this out and see if you can successfully upgrade from 2.3 and run database encoding async upgrade?

All synchronous upgrade scripts have been removed.

Scripts that were altering the database have been moved to phinx database
migrations.

Security settings defaults have been moved to async script to allow
admins access upgrade.php during upgrade process.

Async upgrade to convert database and table encoding to utf8 has been
added.

Database is now seeded before running upgrade tests on Travis.

Async upgrades now run during upgrade tests on Travis.
@hypeJunction
Copy link
Contributor Author

@mrclay I am running out of patience with the encoding async upgrade. See if you can make it work. If it doesn't just remove it, the rest should be functional, and will fix issues with subtype migrations.

@hypeJunction
Copy link
Contributor Author

Looks like it finally worked. Someone do a manual test and merge this idiot.

@jdalsem
Copy link
Member

jdalsem commented Aug 9, 2017

https://travis-ci.org/Elgg/Elgg/jobs/262639154#L1180 should the title of the async upgrade be a readable thing?

@hypeJunction
Copy link
Contributor Author

It should be, not sure why translations are not loaded. I am way passed the stage where I care.

@jdalsem
Copy link
Member

jdalsem commented Aug 9, 2017

could you try to fix it? i am now doing the manual test

@hypeJunction
Copy link
Contributor Author

I honestly don't care about translation strings at this point. I have spent way too much time on this.

@jdalsem
Copy link
Member

jdalsem commented Aug 9, 2017

could do a follow up ticket for the translations

@jdalsem
Copy link
Member

jdalsem commented Aug 9, 2017

Ok did a test:

  1. Fresh 2.0 Elgg install (no additional data faked in)

  2. Switched to 2.3 branch + composer install

  • Async Guid column upgrade appeared but after running would not dissapear. Expect something wrong in detection if it still needs to be executed
  1. Switched to hypejunction/migrations branch + composer install

Site operational... no other issues, so looks good

@jdalsem
Copy link
Member

jdalsem commented Aug 9, 2017

anything else you would like to see tested?

@hypeJunction
Copy link
Contributor Author

Nope, all good then

@jdalsem jdalsem merged commit 38b5f58 into Elgg:master Aug 9, 2017
@hypeJunction hypeJunction deleted the migrations branch August 9, 2017 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants