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

Upgrade to Django 1.8 #3525

Merged
merged 30 commits into from Jan 5, 2016

Conversation

Projects
None yet
4 participants
@robhudson
Member

robhudson commented Sep 28, 2015

@jezdez / @groovecoder : This is now using django-jinja and I open it up for help to finish off the remaining Django 1.8 failures.

Namely:

  • ValueError: save() prohibited to prevent data loss due to unsaved related object 'based_on'.
  • ValueError: save() prohibited to prevent data loss due to unsaved related object 'current_revision'.
  • ValueError: save() prohibited to prevent data loss due to unsaved related object 'parent_topic'.
  • ValueError: dictionary update sequence element #0 has length 0; 2 is required

@robhudson will take:

  • TemplateDoesNotExist: users/email/welcome/plain.ltxt
  • TypeError: __init__() takes exactly 2 arguments (1 given)

I also see the following deprecation warnings in the console:

  • /home/vagrant/src/vendor/src/django-cacheback/cacheback/tasks.py:5: RemovedInDjango19Warning: django.utils.importlib will be removed in Django 1.9.
  • /home/vagrant/src/vendor/src/django-soapbox/soapbox/models.py:42: RemovedInDjango19Warning: Model class soapbox.models.Message doesn't declare an explicit app_label and either isn't in an application in INSTALLED_APPS or else was imported before its application was loaded. This will no longer be supported in Django 1.9.

@robhudson robhudson added the not ready label Sep 28, 2015

@groovecoder

This comment has been minimized.

Member

groovecoder commented Sep 29, 2015

Travis makes it look like django-nose is a library that needs update for 1.8? 😉

@robhudson robhudson added the back end label Sep 29, 2015

@groovecoder

This comment has been minimized.

Member

groovecoder commented Oct 6, 2015

Rebased, updated Makefile, and updated PR description to list the most common errors in the test output thus far.

@groovecoder

This comment has been minimized.

Member

groovecoder commented Oct 6, 2015

After poking into DjangoTranslation and tower, I'm starting to think it may be easier to completely re-do our translation code. DjangoTranslation got a big refactor in 1.8.

@groovecoder

This comment has been minimized.

Member

groovecoder commented Oct 12, 2015

After today's backend meeting, I wonder if we could merge & ship these commits with Django 1.7?

@groovecoder

This comment has been minimized.

Member

groovecoder commented Nov 19, 2015

Re: ValueError: save() prohibited to prevent data loss due to unsaved related object ... it's this change in 1.8.5 to raise this error when saving a model instance with a related un-saved model instance.

We do this in a few places, especially where we wrap many db operations in transaction.set_autocommit(False) ...

For the others, I'm going to try simply moving the related model (i.e., Revision/Document) operations to occur after the the save().

@groovecoder

This comment has been minimized.

Member

groovecoder commented Nov 19, 2015

I've got a commit with re-ordered save operations that fixes the ValueErrors. I opened a pull request to this branch to get feedback on the approach before committing it here.

@groovecoder

This comment has been minimized.

Member

groovecoder commented Nov 23, 2015

Pushed a couple commits from #3668 here to fix some more of the ValueErrors.

@robhudson

This comment has been minimized.

Member

robhudson commented Nov 24, 2015

@groovecoder Branch is updated and rebased with your work which fixed the save() ValueErrors. 🎉

@jezdez The dict ValueErrors seem to be related to how we're subclassing django-rest-framework's TemplateHTMLRenderer? Since you built that do you think you could take a look at determine what it is expecting there? At first I thought it had something to do with our jinja2 instrumented renderer in kuma/core/tests/__init__.py but when I removed that completely the error was still producing.

Tomorrow I'll dig into that TemplateDoesNotExist error.

@jezdez

This comment has been minimized.

Member

jezdez commented Nov 24, 2015

@robhudson Sure, I'll take a look

@stephaniehobson

This comment has been minimized.

Contributor

stephaniehobson commented Dec 14, 2015

A warning in foreman start I don't see recorded here yet:
/home/vagrant/src/vendor/src/django/django/core/management/base.py:260: RemovedInDjango19Warning: "requires_model_validation" is deprecated in favor of "requires_system_checks".

@stephaniehobson

This comment has been minimized.

Contributor

stephaniehobson commented Dec 14, 2015

I don't know if this is specific to this branch but the revision history is out of order:

screen shot 2015-12-14 at 13 26 01

@stephaniehobson

This comment has been minimized.

Contributor

stephaniehobson commented Dec 14, 2015

Caching seems a bit stickier? I needed to manually force-refresh after editing but I think we have open bugs for this on prod so I'm not sure it's a new problem?

@stephaniehobson

This comment has been minimized.

Contributor

stephaniehobson commented Dec 14, 2015

I can't seem to move pages around :( here's the gist from one attempt: https://gist.github.com/stephaniehobson/50c5c306a221025ada99

@robhudson

This comment has been minimized.

Member

robhudson commented Dec 14, 2015

I can't seem to move pages around :( here's the gist from one attempt

It seems that you may have 2 users in your database with the same email.

@stephaniehobson

This comment has been minimized.

Contributor

stephaniehobson commented Dec 15, 2015

Hm. That's possible.

@stephaniehobson

This comment has been minimized.

Contributor

stephaniehobson commented Dec 15, 2015

The page move problem was indeed the duplicated email addresses.

@robhudson

This comment has been minimized.

Member

robhudson commented Dec 15, 2015

@stephaniehobson wrt revision history being out of order... Looking at the code it looks like it always shows the current revision first and the historical revisions after. Is that different than what you're seeing?

@robhudson

This comment has been minimized.

Member

robhudson commented Dec 17, 2015

I compared the revision history page's SQL on master and this branch and the SQL was identical (for the queries that build the revision history list).

robhudson and others added some commits Nov 9, 2015

Bug 1225181 - Configure django-jinja
This also moves jinja2 templates to a jinja2 subfolder.
Bug 1225181 - Replace jingo in python code
This replaces code to use Django and its template engines and also
moves the jinja2 helpers to where django-jinja will load them inside
the Django apps.
Bug 1209185 - Add User migration
This adds a migration since the underlying User subclass has changed.

groovecoder and others added some commits Nov 19, 2015

bug 1209185 - re-order related model saves
Django 1.8.5 raises ValueError when calling .save() on a model
object with un-saved related objects. When we move or revert a
document, we need to re-order our related model save() operations
so we keep data integrity per Django 1.8.5 practice.
Bug 1209185 - Use native Jinja2 Template class from django-jinja.
This does:

- Unpacks Django Context instances to be dicts
- Sends template_rendered signals for testing purposes
Bug 1057293 - Refactored Python requirements.
- moved requirements files into separate files
- updated docs
- start to use peep

@jezdez jezdez removed the not ready label Jan 5, 2016

@robhudson

This comment has been minimized.

Member

robhudson commented Jan 5, 2016

r+ 💥

jezdez added a commit that referenced this pull request Jan 5, 2016

@jezdez jezdez merged commit f5db952 into master Jan 5, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jezdez jezdez deleted the django-1.8 branch Jan 5, 2016

@jezdez

This comment has been minimized.

Member

jezdez commented Jan 5, 2016

26tpdwmm4jyclgxtq

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