Clean up migrations and schema

[sirMackk]

As per my message here: https://groups.google.com/forum/#!topic/plots-dev/QkKfqKStq88
I want to take a go at cleaning up the migrations and schema, which should provide these two benefits:

1. Easier and faster project setup - it should only take a couple of minutes from git clone to rake test or rails s to run smoothly.
2. Easier and faster project setup for production purposes eg. if the project switched servers and such.

At the moment, the best solution I've found is to add a migration that will add missing tables/columns as well as add/remove columns from what's already in the migrations.
This will update the schema to reflect the used database tables and it should allow for a smooth running application and tests. Finally, it'll shift development over to using migrations exclusively.

By the way, do you guys think the migration should additionally remove the unused tables or should they be left as they are?

[jywarren]

I think we should use a migration to remove old tables... but can we first check if they exist? That way the migrations will work for both a bootstrapped and a Drupal legacy database. Also, perhaps the drupal table dropping migration could go at the current timestamp but the original one could be based on this one, which was originally an attempt to manually reconstruct the Drupal schema in the first migration?

https://github.com/publiclab/plots2/blob/master/db/migrate/20120101000000_drupal_schema.rb

[sirMackk]

@jywarren That's awesome and easily doable and it'll help keep things tidy. Thanks!

[jywarren]

awesome.

On Tue, Apr 15, 2014 at 2:38 PM, sirMackk notifications@github.com wrote:

@jywarren https://github.com/jywarren That's awesome and easily doable
and it'll help keep things tidy. Thanks!

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/34#issuecomment-40518019
.

[sirMackk]

I created a migration on my fork here that sits nicely on top of all the previous migrations and allows both the dev server and tests to run smoothly after doing a fresh rake db:create and rake db:migrate.
Since the original Drupal schema migration is untouched, I didn't have to check before dropping any tables.
I didn't submit a pull request yet as I wanted to ask you whether this will work alright in production? Or does the production server need to check if tables exist before dropping/creating them? Thanks :)

[jywarren]

Well, I'm imagining someone coming along with an empty database... vs. the
empty.sql file. Will this work for both scenarios? I can test it out, of
course, but just to be clear.

On Wed, Apr 16, 2014 at 2:48 PM, sirMackk notifications@github.com wrote:

I created a migration on my fork herehttps://github.com/sirMackk/plots2/commit/088facdbda0900d591a0771a17c721e53d7e1d6bthat sits nicely on top of all the previous migrations and allows both the
dev server and tests to run smoothly after doing a fresh rake db:createand rake
db:migrate.
Since the original Drupal schema migration is untouched, I didn't have to
check before dropping any tables.
I didn't submit a pull request yet as I wanted to ask you whether this
will work alright in production? Or does the production server need to
check if tables exist before dropping/creating them? Thanks :)

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/34#issuecomment-40636864
.

[sirMackk]

It'll work for the empty database scenario, but I didn't check it against the empty.sql file scenario. I'll make it work for both and report back :). Thanks for clearing that up for me.

[jywarren]

no, thank you! Perhaps we can ping Andrey to test it out, since he was
trying to get the DB cold-started.

On Wed, Apr 16, 2014 at 3:11 PM, sirMackk notifications@github.com wrote:

It'll work for the empty database scenario, but I didn't check it against
the empty.sql file scenario. I'll make it work for both and report back :).
Thanks for clearing that up for me.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/34#issuecomment-40639633
.

[sirMackk]

I just sent you a pull request. The migrations now work for both scenarios and the tests pass as well. I tested out the application hands-on as well and all seems to be well, although I have to say there might be places/features I'm not aware of yet and haven't tested.

I wonder what @aawray thinks about this, I hope it will help a bit with getting things up and running.

[jywarren]

Tested (whee!) And merged, many thanks!
On Apr 19, 2014 12:22 PM, "sirMackk" notifications@github.com wrote:

I just sent you a pull request. The migrations now work for both scenarios
and the tests pass as well. I tested out the application hands-on as well
and all seems to be well, although I have to say there might be
places/features I'm not aware of yet and haven't tested.

I wonder what @aawray https://github.com/aawray thinks about this, I
hope it will help a bit with getting things up and running.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/34#issuecomment-40875174
.

[sirMackk]

My pleasure!

[amaaov]

Thanks! Nice stuff! 👍

[jywarren]

OK, just to keep this in one place, I'm reopening here. I believe there were a couple issues with the pull request which I didn't catch.

The profile_values, profile_fields, and uploads tables were created in the latest migration you added, with a :force => true, which I believe overwrote all profile bios. I'm still checking if it overwrote any file uploads. We have very thorough nightly backups and I also backed up the db before running the migrations, so no data is at risk, so don't worry. But I believe these table creations ought to be unnecessary, or at least they should be wrapped in a unless table_exists :profile_values statement.

Thoughts?

[sirMackk]

Ah, you are entirely correct! I can't believe I let that slip, but it's my fault. It shouldn't have deleted any actual uploaded files, but it probably deleted the database entries related to them. Great news that you did a backup before running the migrations.

I removed the :force => true from the migration and wrapped them in unless table_exists? statements, so this won't happen again. I upped the code here, but I have one more question:

I thought a little bit more about the :options=>'ENGINE=MyISAM' parts. I used those because they're present in the first migration and I assumed that it's related to the project coming from a Drupal app. However, if I remember correctly, this engine is only available on MySQL and these migrations won't work on Sqlite or Postgresql. Do you think I should investigate this further and perhaps remove these fragments?

[amaaov]

@sirMackk

this engine is only available on MySQL and these migrations won't work on Sqlite or Postgresql.

True. I think removing ENGINE option won't produce any harm, I don't remember any MyISAM-specific lines in code.

[jywarren]

Thanks! Yes, we prefer InnoDB, which I think is the default... the
remaining MyISAM tables are just a drupal legacy issue. Awesome... want to
PR?

On Thu, Apr 24, 2014 at 12:41 PM, Andrey notifications@github.com wrote:

this engine is only available on MySQL and these migrations won't work on
Sqlite or Postgresql.
True. I think removing ENGINE option won't produce any harm, I don't
remember any MyISAM-specific lines in code.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/34#issuecomment-41302924
.

[amaaov]

@jywarren

InnoDB, which I think is the default

It's default for MySQL servers with version higher than 5.5.5, if no changes made in server configuration file.

[sirMackk]

@jywarren Great! I made the alterations and submitted the PR.
@aawray - Thanks for the valuable information, I'll remember it 😃