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

Fix data migrations #1335

Merged
merged 12 commits into from
May 3, 2015
Merged

Fix data migrations #1335

merged 12 commits into from
May 3, 2015

Conversation

eugenk
Copy link
Member

@eugenk eugenk commented Apr 29, 2015

This moves some data-changing migrations from db/data/ to db/migrate/ because

  • executing them shouldn't take too long, so they can be executed synchronously
  • they don't need complex methods
  • they are needed for ontohub to work properly

The migrations have been adjusted to directly work on the database and to not depend on model methods. This adds some database queries, but they are only called once so the extra execution time shouldn't hurt.

The `select_attributes` method shall provide an easy way to directly
select cells from the database.

The `update_attributes!` method shall provide an easy way to directly
update cells in the database.

Except from the `id` method, no model methods are involved.
With `record.update_attributes!`, `save!` is called and may fail because
methods are missing or validations fail. We need to call `save!` only if
the record has not been persisted yet.

Otherwise the column should be changed directly, without invocation of
callbacks or validations.
Putting the data migration together with the schema migration, it could even be
improved. Now it creates/uses the correct prover.
The link to the ontology already needs to be established in the first migration.

Also, the migrations create a ProofAttemptConfiguration, which triggers
the before_create callback. They must not assume that the columns have
been created.
@eugenk
Copy link
Member Author

eugenk commented Apr 29, 2015

I don't know who to assign to this pull request. This is connected to both, ontohub in general and the reasoning milestone of my master's thesis.

@eugenk
Copy link
Member Author

eugenk commented Apr 29, 2015

To test the migrations, I recommend to

  1. Check out 7e216f8 - This has proving functionality, but is before all the moved migrations.
  2. rake db:migrate:clean and rake db:seed
  3. Prove something - Proofs are not in the seeds in this commit.
  4. Check out this branch
  5. Migrate

@tillmo tillmo self-assigned this May 3, 2015
@tillmo
Copy link
Member

tillmo commented May 3, 2015

Sometimes code is duplicated from models (or lib) into the migrations in order to ensure that they work independently of whether this code is present. Looks a bit odd, but seems to be necessary. 👍

eugenk added a commit that referenced this pull request May 3, 2015
@eugenk eugenk merged commit 005b9d7 into staging May 3, 2015
@eugenk eugenk deleted the fix_data_migrations branch May 3, 2015 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants