FreedomCoop / valuenetwork

Fork coming from NRP-Sensorica to use and work for FREEDOM COOP
http://fair.coop
GNU Affero General Public License v3.0
31 stars 12 forks source link

Migrations in different branches #137

Closed bhaugen closed 7 years ago

bhaugen commented 8 years ago

Lynn reminds me that we have had trouble with them in the past, using South. We hope that the new django migrations are better, but we don't know for sure. So we should try to merge some branches that each have migrations and see how it works before merging back into master.

I know the new request_actions branch has a work migration and has not yet been merged into master. I'll look around for another branch with work migrations and merge them both into another new test branch to see if I detect any problems. If you know of another such branch, let me know here. Otherwise, I'll find or create one.

XaviP commented 8 years ago

If the migrations are for creating new models, maybe we can commit them directly in master in the begining of development (this requires a good model dessign), and then merge from master to the branch. But I think is too late for this, isn't it?

bhaugen commented 8 years ago

But I think is too late for this, isn't it?

Yup. The project and request_actions branches both have migrations. I'll create a new branch and merge both of them in and run migrate and see what happens.

I think they are both low risk and we could merge them into master, but I'll merge master into my new test branch first.

XaviP commented 8 years ago

Another possible solution is not pushing (or erase from branches) the migration files, and when merging to master (or to the test-branch you said) download a sync db from test ocp and run .manage.py makemigrations to produce the migration files syncronized with the test and prod ocp dbs. And then push the migration files and pull from test and prod. I don't know if it's well explained. (edited)

fosterlynn commented 8 years ago

Let's not worry about it yet. I'm guessing that they made that work when they put South into Django. And Bob will find out when he tests.

bhaugen commented 8 years ago

Here's what I am learning: basically, the django migrations were developed by the same person who create South, who learned from all the grief South users had with duelling migrations.

https://docs.djangoproject.com/en/1.8/topics/migrations/#version-control

Because migrations are stored in version control, you’ll occasionally come across situations where you and another developer have both committed a migration to the same app at the same time, resulting in two migrations with the same number.

Don’t worry - the numbers are just there for developers’ reference, Django just cares that each migration has a different name. Migrations specify which other migrations they depend on - including earlier migrations in the same app - in the file, so it’s possible to detect when there’s two new migrations for the same app that aren’t ordered.

When this happens, Django will prompt you and give you some options. If it thinks it’s safe enough, it will offer to automatically linearize the two migrations for you. If not, you’ll have to go in and modify the migrations yourself - don’t worry, this isn’t difficult, and is explained more in Migration files below.

https://docs.djangoproject.com/en/1.8/topics/migrations/#migration-files

(Which doesn't tell me much except that it is fine to hand-edit the migration files.)

I'm doing some tests next.

bhaugen commented 8 years ago

Migration merge test:

I created a new branch, migration_test, and merged the project and request_actions branches into it. (Both of them had a migration 7.)

Here's what happened:

./manage.py migrate

CommandError: Conflicting migrations detected (0007_auto_20160831_1907, 0007_auto_20160830_2127 in work).
To fix them run 'python manage.py makemigrations --merge'

Merging work
  Branch 0007_auto_20160831_1907
    - Add field state to membershiprequest
    - Raw Python operation
  Branch 0007_auto_20160830_2127
    - Alter field parent on agenttype
    - Change Meta options on location
    - Alter field state on agentassociation
    - Alter field association_behavior on agentassociationtype
    - Alter field percentage_behavior on valueequation
    - Alter field digital_currency_tx_state on economicevent
    - Create model Project
    - Create model SkillSuggestion
    - Alter field joining_style on project
    - Alter field visibility on project

Merging will only work if the operations printed above do not conflict
with each other (working on different fields or models)
Do you want to merge these migration branches? [y/N] y

Created new merge migration /home/bob/.virtualenvs/fc18x/valuenetwork/work/migrations/0008_merge.py

./manage.py migrate

Everything seems to be ok, although I will do more testing. I'm thinking I should push this branch and people should use it instead of the project and request_actions branches. @fosterlynn @XaviP does that make sense to both of you?

bhaugen commented 8 years ago

Addendum:

work/migrations/0008_merge.py

# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from django.db import migrations, models

class Migration(migrations.Migration):

    dependencies = [
        ('work', '0007_auto_20160831_1907'),
        ('work', '0007_auto_20160830_2127'),
    ]

    operations = [
    ]

Pretty simple.

fosterlynn commented 8 years ago

I'm thinking I should push this branch and people should use it instead of the project and request_actions branches. @fosterlynn @XaviP does that make sense to both of you?

Actually, I don't think so. It will be more complex to think about the whole thing, and those branches are separate now for a reason. Let's just merge the branches in as they are ready to merge, as planned, and everyone will know now how to deal with the migrations.

bhaugen commented 8 years ago

Any other branches with migrations? I'm not seeing any. Let's not do anymore until we decide to merge the migration_test branch into master. I started it from master, so it should merge in easily. The changes that will be visible to coop admins are in the request_actions branch, and I think those are low risk.

bhaugen commented 8 years ago

Our comments crisscrossed. I'll wait to push the migration_test branch, or not push it, awaiting consensus.

bum2 commented 8 years ago

There are some migrations in the fobi branch, let's see if they don't conflict when merging to master... Thanks @bhaugen for getting into the detail of migrations (the strange and delicate part of django).

bhaugen commented 8 years ago

There are some migrations in the fobi branch, let's see if they don't conflict when merging to master...

They will conflict, but the conflicts are probably manageable. The main conflict is the one I ran into between migration 7 in the project and request_actions branches. The migration 7 from project is now both in master and also in the fobi branch. When I merged project and request_actions branches, Django auto-created a migration 8 to merge the two different branch 7s. Now the fobi branch also has a migration 8, too. I did not save the auto-create migration 8, so will need to do that again. I'll discuss the situation with @fosterlynn and we'll figure out what to do tomorrow. I do think we need to resolve this now.

I think the request_actions branch is safe to merge in and deploy.

(the strange and delicate part of django).

Turns out to be not as delicate as the previous South migrations.

bhaugen commented 8 years ago

@fosterlynn thinks we should just merge when we think it fits and see of django deals with the migration merge well enough. If not, scramble.

I think my request_actions branch is read to merge into master, will do so unless somebody strongly disagrees soon. Lynn thinks it would probably better for me to merge into master locally and deal with the migration merge rather than try a pull request which would leave the migration merge to github? somebody else? github won't allow the merge?

fosterlynn commented 8 years ago

the detail of migrations (the strange and delicate part of django)

:smile: great description!

Following on the above: How does this sound for a procedure for if you have a new migration in your branch?

When tested and ready to merge to master:

  1. Get the current master if you don't have it.
  2. Local merge your branch to master.
  3. Migrate locally, do the merge fix as described above by @bhaugen .
  4. Test locally.
  5. Commit new migration.
  6. Push master. Or to do a pull request, create a new branch locally from local master, and do a pull request from that branch.

The reason to do all of this locally is then it is done, nobody has to try to do it on a server, and it is only done once.

bhaugen commented 8 years ago

I just tested it by merging the request_actions branch into master locally. Instead of pushing, though, I'll do a pull request so everybody can look and @XaviP can decide. (Unless Xavi is really out of the loop now...)

fosterlynn commented 8 years ago

OK, I edited the above heavily based on watching Bob test it!

bhaugen commented 8 years ago

See https://github.com/FreedomCoop/valuenetwork/pull/139

fosterlynn commented 8 years ago

It does seem a bit strange though, because now Bob has it already merged to master on his computer, but nobody else will because he did a pull request instead of pushing master directly. Also, what happens if his pull request doesn't get merged before someone else puts another migration into master?

We will have to baby step our way through this I guess.

bhaugen commented 8 years ago

@bum2 don't merge your branch with more migrations into master until we deal with my pull request.

If you are eager to do your merger, merge my pull request first.

XaviP commented 8 years ago

Thanks for testing it. I have no time to test it again, so for me go ahead. Please close branches when they no longer have use (project?).

bhaugen commented 8 years ago

Merged my own pull request and deleted the branch.

XaviP commented 8 years ago

Deployed in testocp. Running migration successfully:

Running migrations:
  Rendering model states... DONE
  Applying work.0006_membershiprequest_state... OK
  Applying work.0007_auto_20160831_1907... OK
  Applying work.0008_merge... OK
XaviP commented 8 years ago

Maybe I'm a little obsessive with this (please tell me if so), but: can we erase project and _requestactions branches? (if everything is merged to master it's secure)

It's easy, just click on the the trash can icon: https://github.com/FreedomCoop/valuenetwork/branches

bhaugen commented 8 years ago

I'll delete the request_actions branch. Should have done that already. I don't know about the project branch, @bum2 might be working on it. So let's ask him.

bhaugen commented 8 years ago

I think comment_extras could be merged into master and deleted, too. I'll take a look and see where it's at.

bhaugen commented 8 years ago

I got a couple of things yet to do with comment_extras. I'll do them today and get that merged in and deleted. But then it will need @bum2 to decide where to put it in the navigation. (Unless we're putting too much on Bumbum's plate...??? in which case I can do it.)

bhaugen commented 8 years ago

Done what I will do today, merged into master locally (had a merge conflict), committed and pushed, and deleted the comment_extras branch.

XaviP commented 8 years ago

Successfully deployed in testocp too.

XaviP commented 8 years ago

New @bum2 merge in master, after installing django-fobi and run (successfully) python manage.py migrate, still seems there's a migration pending:

python manage.py makemigrations

Migrations for 'fobi':
  0004_auto_20160905_2254.py:
    - Alter field plugin_uid on formelement
    - Alter field plugin_uid on formelemententry
    - Alter field plugin_uid on formhandler
    - Alter field plugin_uid on formhandlerentry

The file 0004_auto_20160905_2254.py seems out of scope of git. But it can run python manage.py migrate again without problems:

Running migrations:
  Rendering model states... DONE
  Applying fobi.0004_auto_20160905_2254... OK
bhaugen commented 8 years ago

@bum2 wrote something about that before. I have no idea what is going on here, but will check it out tomorrow morning. Where can I see that migration? Did you just create it locally?

bhaugen commented 8 years ago

It looks like it might be a fobi problem. Like they should have done a makemigrations and merged it into whatever release they put out on pypi. Fobi has been very responsive to issues, so if that's what it is, I expect they will continue to respond.

XaviP commented 8 years ago

Yes, locally. The created migration file is inside virtualenv: env/lib/python2.7/site-packages/fobi/migrations/0004_auto_20160905_2254.py

You can see it here.

bhaugen commented 8 years ago

That looks like a leftover fobi migration. I'll ping them in the morning,

XaviP commented 8 years ago

Oh, sorry, I already did it: https://github.com/barseghyanartur/django-fobi/issues/35

bhaugen commented 8 years ago

Ok, good. Let's wait for the response.

XaviP commented 8 years ago

Created a pull request in django-fobi to easy the work of developer: https://github.com/barseghyanartur/django-fobi/pull/36

bhaugen commented 8 years ago

You're very kind...and helpful and friendly.

bhaugen commented 8 years ago

Is the same migrations? https://github.com/barseghyanartur/django-fobi/blob/master/src/fobi/migrations/0003_auto_20160517_1005.py

XaviP commented 8 years ago

I don't think so: https://github.com/barseghyanartur/django-fobi/pull/36/files

bhaugen commented 8 years ago

Ok, I see the diffs.

XaviP commented 8 years ago

New django-fobi release with our issue fixed: https://github.com/barseghyanartur/django-fobi/releases/tag/0.6.8

bhaugen commented 8 years ago

Nice!

XaviP commented 8 years ago

Not really fixed: https://github.com/barseghyanartur/django-fobi/issues/37

bum2 commented 8 years ago

I have a problem: I was trying to upgrade fobi in testocp, after being able to upgrade it in local (needed sudo), but it pushed django version to 1.10 (by the way, the site was working and don't found any error in some small testing). After the fobi upgrade i install again django 1.8 (because the minimum for fobi is just 1.4.x) and the site works also, but if i make ./manage.py migrate then it says not found django_comments import, and if i install it with "sudo pip install django-contrib-comments' then it complains about pinax_theme_bootstrap_account import, so i stopped. I'm not good at pip's or sysadmin, and i think some kind of python path is changed by the fobi upgrade script, so now it don't find the libraries now... can you @XaviP please take a look at this in testocp?

XaviP commented 8 years ago

I think the better way to upgrade this package is uninstall and install it again:

pip uninstall django-fobi
pip install django-fobi==0.6.8

This way doesn't try to update all dependencies (django). If you have any problems with the buggy migration file in django-fobi, you'll have to remove manually after update. In my case:

rm env/lib/python2.7/site-packages/fobi/migrations/0004_auto_20160905_2254.py
rm env/lib/python2.7/site-packages/fobi/migrations/0004_auto_20160905_2254.pyc

Yes, before touching testocp in this way, I always clone it in local (reinstall and clone db) and try to do it. I think in your case, you can reinstall from scratch, erasing all env/ dir and downloading db from testocp.

XaviP commented 8 years ago

I've updated django-fobi in testocp, so now it's in version 0.6.8. I had to restore db from before to install 0.6.6, so we've lost all test data from few days ago (fobi forms and the join requests) Now the migrations files are sync with the package repo: 0001_initial.py 0002_auto_20150912_1744.py 0003_auto_20160517_1005.py 0004_auto_20160906_1513.py (see here the files in repo)

IMPORTANT: Don't run makemigrations in testocp until release and update of version 0.6.9

XaviP commented 7 years ago

New release v0.9.4 https://github.com/FreedomCoop/valuenetwork/releases/tag/v0.9.4 Closing as the merge of differents branches has been achieved.