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

Upgrading django 1.9 #4

Open
XaviP opened this issue Mar 1, 2017 · 15 comments
Open

Upgrading django 1.9 #4

XaviP opened this issue Mar 1, 2017 · 15 comments

Comments

@XaviP
Copy link
Contributor

XaviP commented Mar 1, 2017

As pointed here, maybe it's necessary to upgrade django earlier in order to benefit from the new features of 1.9 version in the refactoring (Abstract Base Classes include Foreign Keys, and maybe others).

Workflow (can be easily converted into a script), to check upgradings:

cd [install dir]
git clone https://github.com/django-rea/nrp.git
virtualenv env
cd nrp
source ../env/bin/activate
pip install --upgrade pip setuptools
vim requirements.txt # edit to the desired django version
pip install -r requirements.txt --trusted-host dist.pinaxproject.com
pip install Image
python manage.py check
python manage.py makemigrations
python manage.py migrate
python manage.py test django_rea.valueaccounting.tests
python manage.py createsuperuser
python manage.py runserver

In python manage.py check the errors arise. Studing them and correcting code to finally migrate and pass the tests.

@ghost
Copy link

ghost commented Mar 1, 2017

Perhaps we'd use tox.

Here's an example of a tox.ini file of a django project testing everything on many environments.

We would use it to check NRP with the different environments in the travis build.

@XaviP would you like to take a look at it or do you prefer to make changes in the code base but perhaps not those related to the integration builds (I would take a look at tox)?

I don't have experience in tox, but I could help you out, it's a really exciting technology ;) If you take on this task and have any doubts just ping me in telegram and we would even have a Skype call if you have the time.

@XaviP
Copy link
Contributor Author

XaviP commented Mar 1, 2017

First aproach to 1.9 upgrade (edited):

  • ocp/work/__init__.py
    It's not possible to import models inside the root package of an app in django 1.9.
    As pointed in django docs about signals, "It’s recommended to avoid the application’s root module and its models module" and "Signal handlers are usually defined in a signals submodule of the application they relate to" (here more info)
    -> Solved by following previous links.

  • ../env/local/lib/python2.7/site-packages/appconf/utils.py
    Outdated django-appconf: change "from django.utils.importlib import import_module" to "from importlib import import_module"
    This is a dependency problem of pinax-theme-bootstrap-account==1.0b2 in requirements.txt (django-appconf==0.5)
    Solved by including django-appconf==1.0.2 in requirements.txt, after pinax-theme-bootstrap-account.

  • account/views.py
    change "from django.contrib.sites.models import get_current_site"
    to "from django.contrib.sites.shortcuts import get_current_site"

  • django_rea/api/views.py and others
    change "from django.utils.datastructures import SortedDict" to "from collections import OrderedDict", and "SortedDict()" to "OrderedDict()":

grep -rl "from django.utils.datastructures import SortedDict" ./ | xargs sed -i s@'from django.utils.datastructures import SortedDict'@'from collections import OrderedDict'@g
grep -rl "SortedDict()" ./ | xargs sed -i s@'SortedDict()'@'OrderedDict()'@g
  • In djangorestframework: "from django.core.handlers.wsgi import STATUS_CODE_TEXT" gives error.
    Upgrading to last djangorestframework.

  • django_rea/api/serializers.py

  File "django_rea/api/serializers.py", line 27, in PlainContextSerializer
    agent_type = serializers.RelatedField()
  File "../env/local/lib/python2.7/site-packages/rest_framework/relations.py", line 97, in __init__
    'Relational field must provide a `queryset` argument, '
AssertionError: Relational field must provide a `queryset` argument, override `get_queryset`, or set read_only=`True`.

Solved with:

grep -rl "RelatedField()" ./ | xargs sed -i s@'RelatedField()'@'RelatedField(read_only=True)'@g

-> But we need to update correctly django_rea/api to last djangorestframework

  • A lot of RemovedInDjango110Warning warnings, but this is for next upgrade :-)
  • Running makemigrations and migrate without problems.
  • Tests pass !!
  • Three more corrections needed with runserver:
  1. 'WSGIRequest' object has no attribute 'REQUEST'
grep -rl "request.REQUEST.get" ./ | xargs sed -i s@'request.REQUEST.get'@'request.GET.get'@g
  1. 'url' is not a valid tag or filter in tag library 'future'
grep -rl "{% load url from future %}" ./ | xargs sed -i s@'{% load url from future %}'@''@g
  1. In account/views.py, class LoginView, def get_context_data: ctx = kwargs
ctx = super(LoginView, self).get_context_data(**kwargs)
  1. External package pinax-theme-bootstrap==2.0.4 uses {% load url from future %} in templates.
    We must upgrade this package to a version that doesn't use it, compatible with dj1.9.

IMPORTANT: django 1.9 in a ssl website requires PyOpenSSL-16.2.0

@bhaugen
Copy link
Contributor

bhaugen commented Mar 2, 2017

we need to update correctly django_rea/api to last djangorestframework

I'll do that either when the django upgrades are all done, or when anybody needs to use the API for something.

@bhaugen
Copy link
Contributor

bhaugen commented Mar 2, 2017

In the meantime, @XaviP , great work once again on upgrading django!

@XaviP
Copy link
Contributor Author

XaviP commented Mar 2, 2017

  • Ok, I will change all "RelatedField()" to "RelatedField(read_only=True)" in django_rea/api.
  • What about pinax-theme-bootstrap-account? Do you think we can update to last version in order to have django-appconf updated? Can this break something?

@bhaugen
Copy link
Contributor

bhaugen commented Mar 2, 2017

What about pinax-theme-bootstrap-account? Do you think we can update to last version in order to have django-appconf updated? Can this break something?

I don't know.

Somebody (was it you or @escanda ) wanted to switch to the supported account app from the version we internalized. Do you intend to do that at the same time? If so, that would break the changes we made to the account app. Some were bug fixes, and some were to get it to work with the work app. We could probably do those some other way. I most likely did the simplest thing that could possibly work.

@ghost
Copy link

ghost commented Mar 2, 2017

What about pinax-theme-bootstrap-account? Do you think we can update to last version in order to have django-appconf updated? Can this break something?

It will break stuff :). I'd perhaps create a new branch in which to start committing your changes related to the upgrade and include externalizing the app as well. A first step (to see up to what degree it breaks stuff, could be to rename the account folder to _account). This way it will use the default pinax dependency, and you can start working from there.

I'd create an app under django_rea.auth extending pinax auth. With signals and hooksets I think most of the functionality can be done without modifying the app itself. Also in the usage page they explain some ways to override behavior.

@bhaugen
Copy link
Contributor

bhaugen commented Mar 2, 2017

@escanda

externalizing the app

I'm for it.

@XaviP
Copy link
Contributor Author

XaviP commented Mar 2, 2017

You can see all these changes in https://github.com/django-rea/nrp/compare/upgrade_django

  • Solving appconf version by including django-appconf==1.0.2 in requirements.txt, after pinax-theme-bootstrap-account.
  • Solving the import of modules in ocp/work/__init.py__ in the 1.9 djangonic way
  • djangorestframework doesn't give errors, but I don't think it works very well.

@bhaugen
Copy link
Contributor

bhaugen commented Mar 2, 2017

djangorestframework doesn't give errors, but I don't think it works very well.

I wouldn't worry about it yet. We'll need to upgrade to their latest version and then do a makeover. But nobody is using the OCP or DEEP API yet, and I think Chris Troutner was the only user of the original valnet AP, and he cut his own fork: https://github.com/christroutner/valuenetwork

@XaviP
Copy link
Contributor Author

XaviP commented Mar 2, 2017

Two more corrections made and pushed:

  • 'WSGIRequest' object has no attribute 'REQUEST'
grep -rl "request.REQUEST.get" ./ | xargs sed -i s@'request.REQUEST.get'@'request.GET.get'@g
  • 'url' is not a valid tag or filter in tag library 'future'
grep -rl "{% load url from future %}" ./ | xargs sed -i s@'{% load url from future %}'@''@g

@XaviP
Copy link
Contributor Author

XaviP commented Mar 7, 2017

The login form in (anonymous) home doesn't appear. Some info:

  • ocp/urls.py: url(r"^$", LoginView.as_view(template_name='account/login.html'), name='home'),
  • account/views.py:222 Class LoginView(FormView)
  • from django.views.generic.edit import FormView

Account app is an internalized app from django-user-accounts==1.0b3
https://github.com/pinax/django-user-accounts

@ghost
Copy link

ghost commented Mar 7, 2017

Fixed in django-rea/nrp@dffebbb

@XaviP XaviP changed the title Upgrading django Upgrading django 1.9 Mar 9, 2017
@XaviP
Copy link
Contributor Author

XaviP commented Mar 27, 2017

External package pinax-theme-bootstrap==2.0.4 uses {% load url from future %} in templates.
We must upgrade this package to a version that doesn't use it, compatible with dj1.9.
Not necessary, the system can avoid the use of this package templates.

@XaviP
Copy link
Contributor Author

XaviP commented Apr 10, 2017

IMPORTANT: django 1.9 in a ssl website requires PyOpenSSL-16.2.0

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

No branches or pull requests

2 participants