Support Django 1.8(.3) #167

awwad · 2016-01-12T01:44:34Z

Currently, SeattleTestbed/clearinghouse:master runs on Django version 1.6, and does not support Django 1.7 or later. The changes here allow us to run on Django versions through at least Django 1.8.3 (likely through 1.8.x).

See Django deprecation/removal timeline for some of the changes that affected our codebase. In particular, I dealt with:

legacy db transaction control removal
removals of content_type argument support
changes to app loading procedure
switch to migration-based db model
(fill in rest)

Some changes were made by user Kaushik1091, and some by me.

Testing:
The Clearinghouse installation procedure is here. While it is updated, Django 1.7+-exclusive instructions are not noted there (In particular, instead of manage.py syncdb, you should employ make manage.py makemigrations followed by manage.py migrate).
Existing code in the pull request has been tested by me on an Ubuntu VM running Django 1.8.3 (also a bit on 1.7 and 1.6.3, though less thoroughly). The clearinghouse website appears to function fully. I've created users, downloaded keys, downloaded donator installers and installed them, seen the donations be registered and carved up by the clearinghouse, requested vessels, and received them. More testing is merited, but it seems to be functional to my eyes.

At present, some of the comments are a bit sloppy or lacking.

This commit contains changes required while upgrading the Django from v1.6.7 - v1.8.3 - Deprecated @transaction.commit_manually usage in common/maindb.py - Changed as suggested in: https://realpython.com/blog/python/transaction-management-with-django-1-6/ - Creating a ModelForm without either the 'fields' attribute or the 'exclude' attribute is prohibited; - Added fields = "__all__" to all the ModelForms - Changed the referncing style in many files (forms.py, admin.py, maindb.py, views.py) to be consistent and complying with the recommended format of version>1.6 - The server runs without any errors, but with some "loud warnings" which pertain to projected deprecated modules in v1.9 Conflicts: website/html/forms.py website/html/views.py

Few changes in models and corresponding migrations Conflicts: website/control/admin.py website/control/models.py website/html/forms.py website/html/templates/accounts/accounts_base.html website/html/templates/accounts/experimentregistration.html website/html/templates/accounts/register.html website/html/templates/common/geni_base.html website/html/views.py website/populate_sensibility.py

Passing mimetype to HttpResponse is deprecated and removed in Django 1.7 per django/django@8eadbc5” content_type should be used instead. Changed two HttpResponse calls here.

- Had to switch back to from clearinghouse.x.y import z rather than from x.y import z. Was like that in master branch of main repo; not sure why it was changed (probably a good reason…?), but doesn’t work for me otherwise.

…uiltin overload

…alone scripts

…ary.

…e_db_nodes.py

awwad · 2016-02-19T16:41:32Z

Fixes #152

awwad · 2016-03-11T18:36:30Z

Cleaned up :P

lukpueh

I reviewed all the code changes. Most of it looks good, but I did suggest a couple of minor modifications and clarifications.

Also I installed Clearinghouse in a Ubuntu machine, clicked through the pages, created a user and downloaded an installer, which all seemed to work, without any suspicious log messages.

I did find a bunch of RemovedInDjango19Warning and RemovedInDjango110Warning though, but I guess we can address these in a new PR.

lukpueh · 2016-10-07T20:35:27Z

backend/backend_daemon.py

-  django.db.transaction.enter_transaction_management()
+  # Beginning of part of fix for Issue #152
+  # 1. Remove enter_transaction_management() line (Functionality removed in
+  #    Django 1.8. Expecting autocommit to work in its absence.)


We should either actually remove the line or say "Commented out".

enter_transaction_management indeed does not exist in 1.8, and it was already labeled "Deprecated private API" in 1.6, that's probably why the removal isn't even mentioned in the Deprecation Timeline.

Autocommit (default), which "wraps every SQL query in its own transaction", should do the trick as mentioned.

Fixed in c849e07.

lukpueh · 2016-10-07T20:48:10Z

backend/backend_daemon.py

+
+  # If we are in a django version that has django.setup() (1.7+?), run django.setup()
+  # This is needed for django compatibility.
+  # So that it works, undo the repy custom patching of the type builtin so that django can use it. We'll turn it back on after.


Why does django.setup() have to import safe.py? I suppose it's not just in order to first override type and then use the backed up original type. Maybe the comments could be more specific?

Also, let's not worry about Django versions that don't have django.setup. Backwards compatibility is of no use here. Once we require Django 1.8 to be installed for a CH, we cease supporting previous versions. (Side note: The point is more difficult to make for the Seattle "client" software where we have a large installed base with an old version of Python.)

Addressed (but not thoroughly checked for relevance) in e85f995.

lukpueh · 2016-10-07T20:49:31Z

backend/backend_daemon.py

+    django.setup()
+    __builtins__.type = temp_type
+  # end of part of fix for Issue #152
+


DRY! Maybe we could put this mysterious piece of code into a function. See comment above for why it's mysterious.

See above --- we require a suitable Django version with django.setup from now on.

Also addressed in e85f995, see above for caveats.

lukpueh · 2016-10-07T20:52:16Z

common/api/maindb.py

@@ -159,7 +159,7 @@ def init_maindb():



-@transaction.commit_manually
+# @transaction.commit_manually


This line can be removed. The commit_manually decorator was removed in Django 1.8

Fixed in e85f995 which also fixed the module-level docstring.

lukpueh · 2016-10-07T20:59:33Z

common/api/maindb.py

+          geniuser.set_password(password)
+          geniuser.save()
+
+          regenerate_api_key(geniuser)

  except:


Btw. our code-guidelines urge to use Class-based exceptions (I know this is not part of this PR).

(Well-spotted! I've added issue #175 for this.)

lukpueh · 2016-10-07T22:06:05Z

website/html/views.py

@@ -868,7 +873,9 @@ def priv_key(request):
  except LoggedInButFailedGetGeniUserError:
    return _show_failed_get_geniuser_page(request)

-  response = HttpResponse(user.user_privkey, mimetype='text/plain')
+  # Fixed for Issue #152, compatibility with Django 1.8+ requires shift from
+  # mime_type argument to content_type argument.


lukpueh · 2016-10-07T22:06:25Z

website/html/views.py

-  response = HttpResponse(user.user_privkey, mimetype='text/plain')
+  # Fixed for Issue #152, compatibility with Django 1.8+ requires shift from
+  # mime_type argument to content_type argument.
+  response = HttpResponse(user.user_privkey, content_type='text/plain')


d428418 removes that comment, again I think it should be in the commit message.

lukpueh · 2016-10-07T22:06:42Z

website/html/views.py

@@ -885,7 +892,9 @@ def pub_key(request):
  except LoggedInButFailedGetGeniUserError:
    return _show_failed_get_geniuser_page(request)

-  response = HttpResponse(user.user_pubkey, mimetype='text/plain')
+  # Fixed for Issue #152, compatibility with Django 1.8+ requires shift from
+  # mime_type argument to content_type argument.


True too too

d428418 again, comment --> commit message again.

lukpueh · 2016-10-07T22:08:15Z

website/settings.py


  # We have our maindb model defined here, so it must be listed.
-  'clearinghouse.website.control',
+  # 'clearinghouse.website.control.models',


If it must be listed here, why is it commented out?

lukpueh · 2016-10-07T22:08:34Z

website/settings.py

@@ -215,7 +215,7 @@
  #'social_auth.backends.browserid.BrowserIDBackend',
  #'social_auth.backends.contrib.live.LiveBackend',
  # Django default this is always needed and must always be last.
-  'django.contrib.auth.backends.ModelBackend',	    
+  'django.contrib.auth.backends.ModelBackend',      


Two trailing spaces instead of one?

Addressed in 1695579, but there's much more to do.

lukpueh · 2016-10-11T14:20:11Z

website/control/models.py

-  # connection_created only exists with django >= 1.1
-  import django.db.backends.signals
-  django.db.backends.signals.connection_created.connect(_prepare_newly_created_db_connection)
+if django.VERSION >= (1.1):


@aaaaalbert pointed out that we can safely get rid of this version check. If someone uses django <= 1.1 we really can't help. Apart from this line I didn't spot any other exaggerated compatibility legacy.

Fixed in a0b117a.

@lukpueh

@lukpueh points out the this call is mentioned as a "Deprecated private API" in the Django 1.7 docs, https://docs.djangoproject.com/en/1.7/_modules/django/db/transaction/Django 1.8 Also, django/django@0f95608 removes the call from the Django codebase. Django’s default behavior is to run in autocommit mode now, see https://docs.djangoproject.com/en/dev/topics/db/transactions/

See the docstring of the `run_django_setup_using_python_type` function for motivation and details.

aaaaalbert · 2016-10-12T11:30:56Z

I'm working to address @lukpueh's comments now.

Quoting https://docs.djangoproject.com/en/dev/releases/1.6/#deprecated-features-1-6 "Transaction management was completely overhauled in Django 1.6, and the current APIs are deprecated:", going on to list the above decorator.

There doesn't seem to be a need to restore Python's `type` as done in e85f995 .

Also fix indentation.

It related to `gen_edit_user_form` and read "change default field_list from None to '__all__' for Django 1.8 compatibility".

The fix commented relates to Django 1.7's removing of `mime_type` (in favor of `content_type`) in `HttpResponse`. Particularly, the deprecation docs at https://docs.djangoproject.com/en/dev/internals/deprecation/#deprecation-removed-in-1-7 state that "The mimetype argument to the __init__ methods of HttpResponse, SimpleTemplateResponse, and TemplateResponse, will be removed. content_type should be used instead. This also applies to the render_to_response() shortcut and the sitemap views, index() and sitemap()."

lukpueh · 2016-10-17T19:32:51Z

<green light/>

@aaaaalbert addressed the requested changes and more:

opened issue for blanket exceptions
removed obsolete comments
DRYed up type override magic ** and added sensible comments
removed legacy compatibility checking (django 1.1)
fixed indentation
fixed trailing whitespaces

** I think the type override magic deserves a separate ticket

Everything seems to work like before @aaaaalbert's latest changes. (I re-did the same tests and log greping as above)

This undoes e9ef43b (which didn't state its motivation in removing the restriction).

This undoes changes to `website/html/templates/common/geni_base.html` in e9ef43b.

z3r0w0n and others added 11 commits August 20, 2015 13:02

Django 1.7+ compatibility: mimetype -> content_type

d328187

Passing mimetype to HttpResponse is deprecated and removed in Django 1.7 per django/django@8eadbc5” content_type should be used instead. Changed two HttpResponse calls here.

reverting import changes

4e9c826

- Had to switch back to from clearinghouse.x.y import z rather than from x.y import z. Was like that in master branch of main repo; not sure why it was changed (probably a good reason…?), but doesn’t work for me otherwise.

Django 1.8 compatibility fix for forms.py (also doc link update)

2520408

Django 1.8 compatibility - removing enter_transaction_management call

8a41d85

Django 1.7+ compatibility for backend, attempt, workaround for repy b…

897e5f2

…uiltin overload

Django 1.7+ compatibility fix - django.setup in correct spot in stand…

a00c45c

…alone scripts

Removing migrations. Some are Sensibility-specific, rest are unnecess…

cc463bb

…ary.

django 1.7+ compat: adding missed django.setup to polling/check_activ…

fcafb75

…e_db_nodes.py

Updating text of installer download page to reflect Python requirements

af7adc7

awwad self-assigned this Feb 19, 2016

awwad added enhancement newcomer and removed newcomer labels Feb 19, 2016

cleanup of <~> marks and line length for PR

0ef5014

lukpueh requested changes Oct 7, 2016

View reviewed changes

lukpueh reviewed Oct 11, 2016

View reviewed changes

aaaaalbert added 2 commits October 12, 2016 13:04

Restore Python type before django.setup

e85f995

See the docstring of the `run_django_setup_using_python_type` function for motivation and details.

aaaaalbert added 6 commits October 12, 2016 13:43

Remove deprecated @transaction.commit_manually

df9e470

Quoting https://docs.djangoproject.com/en/dev/releases/1.6/#deprecated-features-1-6 "Transaction management was completely overhauled in Django 1.6, and the current APIs are deprecated:", going on to list the above decorator.

Call into django.setup directly.

4f3d50a

There doesn't seem to be a need to restore Python's `type` as done in e85f995 .

Remove Django 1.1 compatibility code

a0b117a

Also fix indentation.

Remove comment that should be a commit message

56af630

It related to `gen_edit_user_form` and read "change default field_list from None to '__all__' for Django 1.8 compatibility".

Remove a bit of trailing whitespace

1695579

aaaaalbert added 3 commits October 18, 2016 11:23

Re-add length restriction for node port number

101289d

This undoes e9ef43b (which didn't state its motivation in removing the restriction).

Revert changes to geni_base.html template

64257db

This undoes changes to `website/html/templates/common/geni_base.html` in e9ef43b.

Add newline to end of file in website/urls.py

3b3488e

aaaaalbert merged commit ac49273 into SeattleTestbed:master Oct 18, 2016

aaaaalbert deleted the fix_django branch October 18, 2016 11:04

lukpueh mentioned this pull request Dec 20, 2016

Update django version in CH and CIB installation docs SeattleTestbed/docs#29

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Django 1.8(.3) #167

Support Django 1.8(.3) #167

awwad commented Jan 12, 2016

awwad commented Feb 19, 2016

awwad commented Mar 11, 2016

lukpueh left a comment

lukpueh Oct 7, 2016

aaaaalbert Oct 12, 2016

lukpueh Oct 7, 2016

aaaaalbert Oct 11, 2016

aaaaalbert Oct 12, 2016

lukpueh Oct 7, 2016

aaaaalbert Oct 11, 2016

aaaaalbert Oct 12, 2016

lukpueh Oct 7, 2016

aaaaalbert Oct 12, 2016

lukpueh Oct 7, 2016

aaaaalbert Oct 12, 2016

lukpueh Oct 7, 2016

lukpueh Oct 7, 2016

aaaaalbert Oct 12, 2016

lukpueh Oct 7, 2016

aaaaalbert Oct 12, 2016

lukpueh Oct 7, 2016

lukpueh Oct 7, 2016

aaaaalbert Oct 12, 2016

lukpueh Oct 11, 2016

aaaaalbert Oct 12, 2016

aaaaalbert commented Oct 12, 2016

lukpueh commented Oct 17, 2016

		@@ -159,7 +159,7 @@ def init_maindb():



		@transaction.commit_manually
		# @transaction.commit_manually

Support Django 1.8(.3) #167

Support Django 1.8(.3) #167

Conversation

awwad commented Jan 12, 2016

awwad commented Feb 19, 2016

awwad commented Mar 11, 2016

lukpueh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaaaalbert commented Oct 12, 2016

lukpueh commented Oct 17, 2016