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

Update to Django 1.8.11 #81

Closed
wants to merge 4 commits into
base: develop
from

Conversation

Projects
None yet
2 participants
@atodorov
Contributor

atodorov commented Mar 23, 2016

This is quite a big change which makes Nitrate compatible with Django 1.8.11. Here's what it does:

  • update the spec and requirements files
  • drop wadofstuff-django-serializers and use the default json serializer. wadofstuff isn't compatible with newer Django and was only used for fixtures which are deprecated anyway
  • drop South - we now use proper Django migrations
  • don't use django.utils.simplejson, instead use json from the standard Python library
  • add app_config for TCMS apps which duplicate names with other Django apps so that we can have unique app labels
  • refactor email validation in tcms.core.contrib.auth.backends because the used regular expression is no longer available from Django
  • django.contrib.comments is now django_comments - this split off of the main Django project
  • django.contrib.contenttypes.generic doesn't exist. Functionality was moved around into other modules
  • admin.py - update syntax for get_queryset() method
  • models.py - BooleanField doesn't have default anymore, so configure it explicitly. IntergerField doesn't accept max_length anymore, ForeignKey(primary_key=True) fields are now OneToOneField,
  • removed initial_data.json fixtures and converted them to Django migrations (named initial_data)
  • removed unittest.json because it is the same as tcms/fixtures/unittest.json

NOTES

  • To convert an existing DB to migrations drop your South migrations and ./manage.py migrate --fake
  • when modifying models don't forget to ./manage.py makemigrations
  • currently tests are broken because they use fixtures which are deprecated. It is also a bad practice to use fixtures with tests, instead tests should create the required data programatically using the model classes
  • All initial data migrations appear to work, except tcms/core/migrations/0001_initial_data.py - it creates default groups but can not assign default permissions. The way Django is designed permissions are created after the DB schema is initialized. I've tried working around it (see inline comments and bug reference) but for some reason the permissions are still not available in the database. Also using integer values for permissions is bad practice, this needs to use permission names.

I will try to resolve the remaining problems and most importantly get the tests running again but that would probably require all tests to be refactored.

Migration to Django 1.9.4 - I've tried that as well but hit problems. In particular

django.core.exceptions.AppRegistryNotReady: Apps aren't loaded yet.

This is due to the fact that Django 1.9 doesn't allow models to be imported in __initi__.py files. I've seen a few which can be refactored to load the required modules/methods elsewhere but the entire xmlrpc/api module depends on importing modules and requires significant work.

@atodorov

This comment has been minimized.

Contributor

atodorov commented Apr 7, 2016

NOTE: I realized this is too invasive and we still don't have properly configured test environment (at least I don't) so please DON'T MERGE this for the moment. I plan to split out various sections out of it in separate pull requests (for example migrate away from simplejson to json, etc) which can be reviewed and accepted more easily. Then we have to deal with DB migrations in a better way.

@tkdchen

This comment has been minimized.

Member

tkdchen commented Jun 11, 2016

Hi, what about the progress now?

@atodorov

This comment has been minimized.

Contributor

atodorov commented Jan 19, 2017

@tkdchen sorry for the late reply. I'm looking again at this PR. Let me know how would you like to proceed with it.

option 1) work to enable test execution, rebase and fix all failures
option 2) merge as is and fix failures after manual testing ? (I'm using a custom fork with this patch and everything appears to be working fine).

@tkdchen

This comment has been minimized.

Member

tkdchen commented Jan 21, 2017

Hi, I have made lots of changes to upgrade the Django to 1.8.14 and other improvements. That is in my branch now, https://github.com/tkdchen/Nitrate/commits/upgrade-django. Major changes are

  • Drop old database migration, and database can be created by migrate
  • Upgrade many dependencies, e.g. django-tinymce, django-uuslug.
  • Use django-preserialize instead of wadofstuff-django-serializers

In my branch, you can see other changes to the codebase. I tested and didn't find big issues. Now, I'm going to merge it to master. And I'm glad to see you are still using Nitrate and have done so many improvement. Would you mind to test my branch and merge your other changes on top of it? Next, I'm going to continue to upgrade to Django 1.9 since it's the version in Fedora now.

@tkdchen

This comment has been minimized.

Member

tkdchen commented Jan 21, 2017

Regarding the tests, current tests is not good enough. I'm also considering to rewrite some of them, even all maybe.

@atodorov

This comment has been minimized.

Contributor

atodorov commented Jan 21, 2017

I don't find the 'upgrade-django' branch in your fork. Can you publish it and I'll give it a try.

EDIT: nevermind I saw you merged the branch. Doing some testing on my end now.

I'll also appreciate of you can merge some of the existing PRs so we don't get that many conflicts and so that it is easier to synchronize our work wrt upgrading Django.

@atodorov

This comment has been minimized.

Contributor

atodorov commented Jan 21, 2017

Closing in favor of #109

@atodorov atodorov closed this Jan 21, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment