Skip to content
This repository has been archived by the owner on Feb 1, 2024. It is now read-only.

Upgrade Django to 2.2 and upgrade Django-related libraries to latest versions #676

Merged
merged 1 commit into from Jul 18, 2019

Conversation

kellyi
Copy link
Contributor

@kellyi kellyi commented Jul 15, 2019

Overview

  • Upgrade Django to 2.2
  • Upgrade some django-related libraries to their latest versions

I dd not upgrade libraries related to dedupe or batch processing, as I don't know how easy it is to test regressions there.

Connects #455

Demo

Screen Shot 2019-07-16 at 8 15 23 PM

Testing Instructions

  • vagrant destroy then ./scripts/setup and verify that everything sets up as it should
  • ./scripts/resetdb and ./scripts/server
  • visit the app on 6543 and verify that the following stuff works:
    • registering (and getting registration email)
    • signing in
    • uploading a list
    • searching for facilities
    • saving profile updates & generating API tokens
  • process the list you uploaded using batch_process and verify that that still works
  • visit your list in the UI and confirm or reject some facility matches if any are available, verifying that that works and also that you can remove list items
  • visit the API docs and verify that they load and that you can use them to test the various endpoints

Checklist

  • fixup! commits have been squashed
  • CI passes after rebase
  • CHANGELOG.md updated with summary of features or fixes, following Keep a Changelog guidelines

@rajadain
Copy link
Contributor

I ran this with Python warnings enabled to see all deprecation warnings:

$ docker-compose run --rm --entrypoint python django -Wa manage.py test

And got the following unique warnings:

/usr/local/lib/python3.7/importlib/_bootstrap.py:219: ImportWarning: can't resolve package from __spec__ or __package__, falling back on __name__ and __path__
/usr/local/lib/python3.7/site-packages/dedupe/blocking.py:30: DeprecationWarning: time.clock has been deprecated in Python 3.3 and will be removed from Python 3.8: use time.perf_counter or time.process_time instead
/usr/local/lib/python3.7/site-packages/dedupe/core.py:267: DeprecationWarning: time.clock has been deprecated in Python 3.3 and will be removed from Python 3.8: use time.perf_counter or time.process_time instead
/usr/local/lib/python3.7/site-packages/dedupe/core.py:280: DeprecationWarning: time.clock has been deprecated in Python 3.3 and will be removed from Python 3.8: use time.perf_counter or time.process_time instead
/usr/local/lib/python3.7/site-packages/dedupe/core.py:295: DeprecationWarning: time.clock has been deprecated in Python 3.3 and will be removed from Python 3.8: use time.perf_counter or time.process_time instead
/usr/local/lib/python3.7/site-packages/itypes.py:2: DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated, and in 3.8 it will stop working
/usr/local/lib/python3.7/site-packages/jinja2/runtime.py:318: DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated, and in 3.8 it will stop working
/usr/local/lib/python3.7/site-packages/jinja2/utils.py:485: DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated, and in 3.8 it will stop working
/usr/local/lib/python3.7/site-packages/past/translation/__init__.py:35: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
/usr/local/lib/python3.7/site-packages/past/types/oldstr.py:5: DeprecationWarning: Using or importing the ABCs from 'collections' instead of from 'collections.abc' is deprecated, and in 3.8 it will stop working
/usr/local/lib/python3.7/site-packages/whitenoise/django.py:49: RemovedInDjango31Warning: The FILE_CHARSET setting is deprecated. Starting with Django 3.1, all files read from disk must be UTF-8 encoded.
/usr/local/lib/python3.7/site-packages/xlrd/xlsx.py:266: PendingDeprecationWarning: This method will be removed in future versions.  Use 'tree.iter()' or 'list(tree.iter())' instead.
/usr/local/lib/python3.7/site-packages/xlrd/xlsx.py:312: PendingDeprecationWarning: This method will be removed in future versions.  Use 'tree.iter()' or 'list(tree.iter())' instead.

which are all in libraries, not in our code, which is good.

@kellyi kellyi force-pushed the ki/upgrade-django branch 2 times, most recently from a71e284 to 9a9b585 Compare July 16, 2019 23:56
@kellyi kellyi changed the title WIP Upgrade Django to 2.2 along with some related libs Upgrade Django to 2.2 and upgrade Django-related libraries to latest versions Jul 17, 2019
@kellyi
Copy link
Contributor Author

kellyi commented Jul 17, 2019

I ran this with Python warnings enabled to see all deprecation warnings:

Cool. It appears that most of these reference Python 3.8, which is ahead of our version (and doesn't exist in a final version until October 2019).

I didn't update Dedupe or anything because that seemed like a separate set of things to test.

@kellyi kellyi requested a review from jwalgran July 17, 2019 00:20
@kellyi kellyi marked this pull request as ready for review July 17, 2019 00:21
@kellyi kellyi force-pushed the ki/upgrade-django branch 2 times, most recently from 33cff61 to 27d48a6 Compare July 18, 2019 14:31
@jwalgran
Copy link
Contributor

jwalgran commented Jul 18, 2019

After rebuilding my VM, I set a value on OAR_CLIENT_KEY and restarted my `./scripts/server

diff --git a/docker-compose.yml b/docker-compose.yml
index a769732..1f477a4 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -25,7 +25,7 @@ services:
         - DJANGO_SECRET_KEY=secret
         - DJANGO_LOG_LEVEL=INFO
         - AWS_PROFILE=${AWS_PROFILE:-open-apparel-registry}
-        - OAR_CLIENT_KEY=
+        - OAR_CLIENT_KEY=clientkey
     build:
       context: ./src/django
       dockerfile: Dockerfile

The django-rest-swaggar UI is not sending the auth header

2019-07-18 10 35 13

There is an open issue
marcgibbons/django-rest-swagger#759

Rolling back to 2.1.1 2.1.2 seems like the most expedient way to resolve the problem.

@jwalgran
Copy link
Contributor

Correction. 2.1.2 is the latest release before 2.2.0.

@kellyi
Copy link
Contributor Author

kellyi commented Jul 18, 2019

I pushed 3706843 which upgrades DRF to 3.10.1 and downgrades DR-Swagger to 2.1.2

Copy link
Contributor

@jwalgran jwalgran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not find any regressions when evaluating these changes on staging.

These are the library diffs I browsed through

azavea/django-amazon-ses@2.0.0...2.1.1/
azavea/django-ecsmanage@1.0.1...1.1.0
django-extensions/django-extensions@2.1.4...2.1.9
Tivix/django-rest-auth@v0.9.3...0.9.5
marcgibbons/django-rest-swagger@2.1.2...2.2.0
jazzband/django-waffle@v0.16.0...v0.17.0
mwarkentin/django-watchman@0.15.0...0.17.0
encode/django-rest-framework@3.9.4...3.10.1

We ended up not upgrading django-rest-swagger because of an incompatibility with DRF TokenAuthentication. Implementing #691 will allow us to upgrade.

I read through the incompatibility sections of the 2.1 and 2.2 Django releases

https://docs.djangoproject.com/en/2.1/releases/2.1/#backwards-incompatible-2-1
https://docs.djangoproject.com/en/2.2/releases/2.2/#backwards-incompatible-2-2

Nothing on these lists affects our application.

@jwalgran jwalgran assigned kellyi and unassigned jwalgran Jul 18, 2019
@kellyi
Copy link
Contributor Author

kellyi commented Jul 18, 2019

Thanks for checking this! Going to squish and then merge.

- Upgrade Django to 2.2
- Upgrade some django-related libraries to the latest versions
- Adjust `DEFAULT_SCHEMA_SETTING` for Django-REST-Framework so that we
continue to use CoreAPI rather than OpenAPI
@kellyi kellyi merged commit d01e625 into develop Jul 18, 2019
@kellyi kellyi deleted the ki/upgrade-django branch July 18, 2019 20:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants