Merge lp:~shevonar/widelands-website/django1.4 into lp:widelands-website

[Shevonar]

I updated Django and all other requirements to newer versions when it was not to much work to migrate to the new version. So some requirements might not be up-to-date, e.g. sphinx-doc which we might not even use in the future.

I fixed all problems I found until now. Please test the new setup on http://wl.shevonar.com to find all remaining bugs. I had no time to test it in depth yet.

[SirVer (sirver)]

Good to see you back in swing, Shevonar. The change LGTM in general - it is also much shorter than I would have expected :). Just one question: do you know if the database schema needs updating for this new version and if yes, how would I do this?

[Shevonar (shevonar)]

Yes indeed there are some changes. First the notification tables changed a little and must be changed manually [1].
Second Django now supports native timezones and therefore the date and time data in the database must be converted to UTC before enabling USE_TZ in the settings. I have not tested it yet but a migration guide can be found here [2]

[1] https://github.com/jtauber/django-notification/issues/60
[2] https://docs.djangoproject.com/en/dev/topics/i18n/timezones/#time-zones-migration-guide

[SirVer (sirver)]

I suggest adding a new document to the repo called transitions which contains revision number + links to the stuff that needs to be done for this revision to work - very similar than what you just wrote. It will make migration a bit easier.

Btw, you have push rights to trunk, right? I do not have to merge this for you, but I can if you want to

[SirVer (sirver)]

I necro this one. Do you think it is worth merging or should we directly try moving to 1.5. I really dislike that idea - so many packages that will need updating.

[SirVer (sirver)]

Setting this to rejected as it is really abandoned right now.

[Hans Joachim Desserud (hjd)]

Hi all. :)

I have been checking in on this merge proposal every now and then to see if there has been some progress. From what I can see the code/dependencies are updated, the only remaining changes are the database migrations, is this correct?

>Do you think it is worth merging or should we directly try moving to 1.5

I think we need to take this gradually. I remember branching this and bumping the dependency to django 1.5 just to check and it seemed more parts of the api had moved around causing failing imports. I didn't look further into it, because I figured we needed to land this first anyways.

While we should do something to sync up with more current releases, simply getting to 1.4 would be huge improvement already. Django 1.4 is an LTS-relese [1] and will therefore get security patches at least until March 2015. I don't think we should stop there, but we would at least be running a release which is currently supported. :)

[1] https://docs.djangoproject.com/en/dev/internals/release-process/#long-term-support-lts-releases

[SirVer (sirver)]

Updating the database though always feels like surgery on the open heart without backup. That is the main reason why we did not update - it is just so risky and time consuming. Especially with Python where you never know if your code is actually broken till it breaks.

I agree to your reasoning that a LTS release is where we should be.

[Gabriel Margiani (gamag)]

> Updating the database though always feels like surgery on the open heart without backup.
Why not dump the database to an sql script for backup, or modify the script to update (which is quite hard with a lot of data in it)?

[SirVer (sirver)]

That is of course what we do. But still: you have a dump file, then you update all the software and hope that they really play as well together as in the tests. Then you run the migration scripts that usually fail somewhere halfway through - you have a database in a weird state then. You pick up the pieces, massage the db so that the update scripts run through. Then you reboot the system and see if anything blows up....

I think there are probably better ways of doing these kind of things, but I do not know them. Sql is just very flawed when it comes to upgrading.

[Hans Joachim Desserud (hjd)]

>Then you run the migration scripts that usually fail somewhere halfway through - you have a database in a weird state then. You pick up the pieces, massage the db so that the update scripts run through.

Well, presumably this is run in some dev/test environment so we can go back and rework the migration scripts to work better. If the migration scripts fail halfway through, they are not finished/complete.

The real work here is making sure we test/exercise enough of the code base to make sure we don't have any necessary migrations we aren't aware of lurking in the corners. Smoketesting is a given here, but may miss some edge cases. I see there are some test suites in the repository, but I don't know they coverage rate of them. The production migration should not take place until we have verified the migration scripts run and that the site works without any problems afterwards.

I'm not familiar with it, but I spotted the dependency on South which seems quite capable of dealing with migrations. (Actually autogenerating the necessary migrations based on changes to the models: http://south.readthedocs.org/en/latest/tutorial/part1.html!) If the only necessary database changes needed are the two mentioned by Shevonar above, it looks rather feasible *knocks on wood*. I think someone should take a closer look at South and try to create some migrations based on the information linked to above.

[Hans Joachim Desserud (hjd)]

I have looked some more into the required migrations.

There's django-notification and changes in one table. I sorta have an approach for this, but it is rather hacky... Basically it would perform the necessary changes described in the issue linked to by Shevonar. Though since we don't define the notification models directly, from what I can see we import them in wiki/models.py, we don't really have an app which owns them. And South (the migration tool) is rather built for running migrations for an app, while we would have a migration for a table which comes from a dependency which we happen to have created. That doesn't sound all that ideal.

(An alternative solution could be to create a patched version of django-notification which includes the necessary migration and depend on that instead. Though at the moment it is not clear to me which point this would be executed when we do an upgrade. (Naturally we would switch back to a non-patched version later down the road)

Somewhat related question: Shevonar, why was the github repo where we pull in django-notification changed? I see the latest commit in the original repo we pointed to has replaced the code with a link to a different (third) repo, but I don't follow this project so I don't know what the differences are. Unless we need something specific, I'm somewhat tempted to simply replace the github url with version 0.2 and fetch it from pypi.

Regarding the timezone thingy, from what I read it would be possible to upgrade to 1.4 without touching the timezone info. We would need to add "USE_TZ = False" and it could continue to work as earlier. I do think we should do this migration, but it doesn't seem to be a prerequisite for upgrading to 1.4 so we might be able to delay it until we are at a more recent version.

SirVer: Btw, what sort of database do we use in production, which may affect how to approach how we deal with the timezones and potential conversion of the data?

According to https://docs.djangoproject.com/en/1.7/internals/release-process/, Django 1.4 will be supported until at least March 2015. That's not too long, but arguably better than no support. I think we need to push this more, because I fear we will run into problems unless we are able to upgrade it. :(

[SirVer (sirver)]

> SirVer: Btw, what sort of database do we use in production, which may affect how to approach how we deal with the timezones and potential conversion of the data?

We use mysql on the server.

> I think we need to push this more, because I fear we will run into problems unless we are able to upgrade it. :(

I agree - but right now we do not seem to have clear steps forward for this.

[Hans Joachim Desserud (hjd)]

>I agree - but right now we do not seem to have clear steps forward for this.

First of all, I feel I should apologize for the delay. The messages back and forth on this merge proposal is quite sporadic. I do care about this issue, but for some reason (though partially because I don't know Django all that well) I find that I never really get round to working on this.

I can say abit about what I found out last time I looked into this. I investigated south for database migrations which I was happy (and surprised) to find that we already use. Seems to at least have been used for pybb/migrations, though I don't know the details nor who used this.

The good news is that South is able to autogenerate migrations based on what it knows about the existing database state when the models.py for a module changes. Though this require that it actually knows about the initial state in order to tell what has changed.

So my battle plan/sketch is as follows:
1. We need to map out the modules and related database tables which will need to be migrated during the upgrade so that we can record the intial/current state for these. (Alternatively record the current state for all tables so that we can more easily work with database migrations in the future)
2. For these tables, South can automatically generate a migration which would create/setup the tables based on how they are defined in models.py. These would be refered to as intial migrations. (I would link to the South documentation on how to do this, but it has been a while since I read about it.)
3. The migrations from 2 should be gathered in a separate merge proposal and pushed to production. There's a command which can be run to perform the migrations. Here's the CRITICAL step: These initial migrations should be faked using --fake [1]. This doesn't actually run the migrations, but stores them in the migrations table to record that they have been run. The reason for doing this is because we are letting the migration system know what exists, if actually run the intial migrations it will assume no such tables exist and attempt to create them (from what I read, possibly clear the data if they do exist!)
4. At this point we are still running the old code, but we are now aware of the current database state. Now we return to this merge proposal and continue working on it. Check out this code on a local machine, review and pull in the updated dependencies for it.
5. With the new version of dependencies and their modules in place, I believe it should be possible to simply run South to generate the necessary migrations to get from the known state in the database to the modules.py defined in these newer dependencies.
6. This should then be thoroughly tested against a copy of the prod database to ensure things still remain working.
7. When rolling out in prod (or for the test in the previous code), it would consist of the following steps:
1) Stop the app.
2) Pull in the latest code changes
3) Run the migrations to get the DB up to date
4) Start the app again.
5) Verify things are working
6) Be happy :)

So that's a plan at least. As you notice it would be split across two independent deployments. The intial migrations...

[SirVer (sirver)]

[regarding south]
I started using this very early in the development of the Page - it was just such a PITA not having a migration schema.

Your battleplan sounds convincing - and a lot of work. However it sets us up for easier updates moving forward in the future. Are you working on step 1?

[Hans Joachim Desserud (hjd)]

> Are you working on step 1?

No. Sorry. :(

As I mentioned briefly in the previous comment I never seem to get around to working on this. I guess partially because it's somewhat intimidating since I haven't used Django/South that much. This is also why I posted the comment, since I didn't knew when/if I would get round to it, but at least then others would know of the sketched plan.

Unmerged revisions

341. By Shevonar on 2013-01-15

Updated requirements to newer versions and fixed related problems

=== modified file 'news/feeds.py'
--- news/feeds.py	2011-08-24 13:09:49 +0000
+++ news/feeds.py	2013-01-15 01:15:26 +0000
@@ -1,7 +1,6 @@
-from django.contrib.syndication.feeds import FeedDoesNotExist
+from django.contrib.syndication.views import Feed, FeedDoesNotExist
 from django.core.exceptions import ObjectDoesNotExist
 from django.contrib.sites.models import Site
-from django.contrib.syndication.feeds import Feed
 from django.core.urlresolvers import reverse
 from widelands.news.models import Post, Category
 

=== modified file 'pip_requirements.txt'
--- pip_requirements.txt	2011-11-13 15:04:36 +0000
+++ pip_requirements.txt	2013-01-15 01:15:26 +0000
@@ -1,20 +1,20 @@
-Django==1.3.1
-wsgiref==0.1.2
--e git://github.com/dcramer/django-sphinx.git#egg=djangosphinx
-docutils==0.7
-PIL==1.1.7
--e svn+http://django-pagination.googlecode.com/svn/trunk/#egg=pagination
--e hg+https://django-tracking.googlecode.com/hg/#egg=tracking
--e git://gitorious.org/python-markdown/mainline.git#egg=markdown
-BeautifulSoup==3.2
--e hg+http://bitbucket.org/ubernostrum/django-registration/#egg=registration
--e svn+http://django-tagging.googlecode.com/svn/trunk/#egg=tagging
--e git://github.com/dcramer/django-ratings.git#egg=djangoratings
-django-threadedcomments==0.5.2
--e svn+http://django-messages.googlecode.com/svn/trunk/#egg=django_messages
--e git://github.com/jtauber/django-notification.git@3f023adf0ce2eafcee744904e2c358792f253721#egg=notification
-django-sphinxdoc==0.3.2
-South==0.7.3
-numpy
-pydot
-ipython
+Django==1.4.3
+wsgiref==0.1.2
+django-sphinx==2.2.4
+docutils==0.10
+PIL==1.1.7
+django-pagination==1.0.7
+django-tracking==0.4.1
+-e git://github.com/waylan/Python-Markdown.git#egg=Markdown
+BeautifulSoup==3.2.1
+django-registration==0.8
+django-tagging==0.3.1
+django-ratings==0.3.7
+django-threadedcomments==0.5.3
+-e git://github.com/mirumee/django-messages.git#egg=django_messages
+-e git://github.com/matiasherranz/django-notification.git@5bc9494785cfb65a5ef04d23daf53e1b6aa3cccc#egg=django-notification
+django-sphinxdoc==0.3.2
+South==0.7.4
+numpy
+pydot
+ipython
\ No newline at end of file

=== modified file 'pybb/feeds.py'
--- pybb/feeds.py	2010-06-13 12:46:44 +0000
+++ pybb/feeds.py	2013-01-15 01:15:26 +0000
@@ -1,4 +1,4 @@
-from django.contrib.syndication.feeds import Feed
+from django.contrib.syndication.views import Feed
 from django.core.urlresolvers import reverse
 from django.utils.translation import ugettext_lazy as _
 from django.core.exceptions import ObjectDoesNotExist

=== modified file 'pybb/urls.py'
--- pybb/urls.py	2012-04-19 19:46:21 +0000
+++ pybb/urls.py	2013-01-15 01:15:26 +0000
@@ -13,7 +13,7 @@
     url('^$', views.index, name='pybb_index'),
     url('^category/(?P<category_id>\d+)/$', views.show_category, name='pybb_category'),
     url('^forum/(?P<forum_id>\d+)/$', views.show_forum, name='pybb_forum'),
-    url('^feeds/(?P<url>.*)/$', 'django.contrib.syndication.views.feed',
+    url('^feeds/(?P<url>.*)/$', 'django.contrib.syndication.views.Feed',
         {'feed_dict': feeds}, name='pybb_feed'),
 
     # Topic

=== modified file 'settings.py'
--- settings.py	2012-05-17 19:28:39 +0000
+++ settings.py	2013-01-15 01:15:26 +0000
@@ -69,6 +69,7 @@
     'django.middleware.gzip.GZipMiddleware', # Remove this, when load gets to high or attachments are enabled
     'django.middleware.common.CommonMiddleware',
     'django.contrib.sessions.middleware.SessionMiddleware',
+    'django.contrib.messages.middleware.MessageMiddleware',
     'django.middleware.csrf.CsrfViewMiddleware',
     'django.contrib.auth.middleware.AuthenticationMiddleware',
     'pagination.middleware.PaginationMiddleware',
@@ -220,6 +221,7 @@
     'django.contrib.auth',
     'django.contrib.contenttypes',
     'django.contrib.sessions',
+    'django.contrib.messages',
     'django.contrib.sites',
     'django.contrib.admin',
     'django.contrib.markup',

=== modified file 'urls.py'
--- urls.py	2012-05-17 19:28:39 +0000
+++ urls.py	2013-01-15 01:15:26 +0000
@@ -24,7 +24,7 @@
     # overwrite registration with own implementation
     url (r'^accounts/register/$', 'mainpage.views.register', name='registration_register'),
     (r'^accounts/', include('registration.backends.default.urls')),
-    (r'^feeds/(?P<url>.*)/$', 'django.contrib.syndication.views.feed', {'feed_dict': feeds}),
+    (r'^feeds/(?P<url>.*)/$', 'django.contrib.syndication.views.Feed', {'feed_dict': feeds}),
 
     # 3rd party, unmodified
     (r'^notification/', include('notification.urls')),

=== modified file 'wiki/feeds.py'
--- wiki/feeds.py	2010-06-13 12:46:44 +0000
+++ wiki/feeds.py	2013-01-15 01:15:26 +0000
@@ -1,4 +1,4 @@
-from django.contrib.syndication.feeds import Feed, FeedDoesNotExist
+from django.contrib.syndication.views import Feed, FeedDoesNotExist
 from django.utils.feedgenerator import Atom1Feed
 from django.contrib.contenttypes.models import ContentType
 from django.core.exceptions import ObjectDoesNotExist

=== modified file 'wiki/views.py'
--- wiki/views.py	2012-09-20 20:03:21 +0000
+++ wiki/views.py	2013-01-15 01:15:26 +0000
@@ -11,7 +11,7 @@
 from django.shortcuts import get_object_or_404, render_to_response, redirect
 from django.utils.translation import ugettext_lazy as _
 from django.contrib.contenttypes.models import ContentType
-from django.contrib.syndication.feeds import FeedDoesNotExist
+from django.contrib.syndication.views import FeedDoesNotExist
 
 from wiki.forms import ArticleForm
 from wiki.models import Article, ChangeSet, dmp