Skip to content

Upgrade to django 3.2#2533

Merged
rtibbles merged 2 commits intolearningequality:unstablefrom
rtibbles:django2.2
May 20, 2021
Merged

Upgrade to django 3.2#2533
rtibbles merged 2 commits intolearningequality:unstablefrom
rtibbles:django2.2

Conversation

@rtibbles
Copy link
Copy Markdown
Member

@rtibbles rtibbles commented Nov 10, 2020

Description

  • Does the minimal required to upgrade to Django 3.2
  • Fixes all tests and updates code for Django 3.2 compatibility
  • Upgrades all django related libraries to newest versions
  • Makes necessary updates to code to handle API changes across versions

@codecov
Copy link
Copy Markdown

codecov bot commented Nov 10, 2020

Codecov Report

Merging #2533 (1c9fb14) into unstable (7558536) will decrease coverage by 1.30%.
The diff coverage is 86.37%.

❗ Current head 1c9fb14 differs from pull request most recent head 0ee0285. Consider uploading reports for the commit 0ee0285 to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##           unstable    #2533      +/-   ##
============================================
- Coverage     86.13%   84.82%   -1.31%     
============================================
  Files           305      182     -123     
  Lines         16455    15034    -1421     
============================================
- Hits          14173    12753    -1420     
+ Misses         2282     2281       -1     
Impacted Files Coverage Δ
contentcuration/contentcuration/api.py 91.80% <ø> (-0.27%) ⬇️
...ntentcuration/contentcuration/db/models/manager.py 90.74% <ø> (-0.31%) ⬇️
...igrations/0001_squashed_0094_auto_20180910_2342.py 100.00% <ø> (ø)
...uration/contentcuration/tests/test_contentnodes.py 94.96% <ø> (+0.35%) ⬆️
contentcuration/search/viewsets/savedsearch.py 79.31% <0.00%> (ø)
...ation/contentcuration/tests/test_rest_framework.py 36.93% <26.92%> (ø)
contentcuration/contentcuration/views/settings.py 74.74% <31.25%> (ø)
contentcuration/contentcuration/decorators.py 56.60% <33.33%> (ø)
contentcuration/contentcuration/views/users.py 64.32% <41.66%> (+0.14%) ⬆️
contentcuration/contentcuration/tasks.py 63.25% <50.00%> (+1.67%) ⬆️
... and 111 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eeeea69...0ee0285. Read the comment docs.

@jredrejo
Copy link
Copy Markdown
Member

As this is a huge change in terms of requirements, could you take a look at #2236 to see if you can include some of the changes from there, so we'd be hitting two targets with one shot?
I think the only part that would need to be added to your code is:

-celery==4.1.1
-redis==2.10.5
+celery==4.3.0
+redis==3.2.0

with its associated dependencies.
If that'd work with python 3.x and Django 2.2 we'd have full support of 3.8 and ,likely, 3.9 too.

@rtibbles
Copy link
Copy Markdown
Member Author

Can do, that seems in line with the big dependency shift!

Copy link
Copy Markdown
Member

@jredrejo jredrejo left a comment

Choose a reason for hiding this comment

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

Tested with python 3.8 and everything seems to work fine... but when clicking on a channel to open its edit modal, I get quite often this error trace:

"HEAD /api/contentnode/ce5297fa2c5247529b9bcc68eec77d97 HTTP/1.1" 200 1067
INFO:django.server:"HEAD /api/contentnode/ce5297fa2c5247529b9bcc68eec77d97 HTTP/1.1" 200 1067
----------------------------------------
Exception happened during processing of request from ('127.0.0.1', 56004)
Traceback (most recent call last):
  File "/usr/lib/python3.8/socketserver.py", line 650, in process_request_thread
    self.finish_request(request, client_address)
  File "/usr/lib/python3.8/socketserver.py", line 360, in finish_request
    self.RequestHandlerClass(request, client_address, self)
  File "/usr/lib/python3.8/socketserver.py", line 720, in __init__
    self.handle()
  File "/datos/le/studio/venv/lib/python3.8/site-packages/django/core/servers/basehttp.py", line 171, in handle
    self.handle_one_request()
  File "/datos/le/studio/venv/lib/python3.8/site-packages/django/core/servers/basehttp.py", line 179, in handle_one_request
    self.raw_requestline = self.rfile.readline(65537)
  File "/usr/lib/python3.8/socket.py", line 669, in readinto
    return self._sock.recv_into(b)
ConnectionResetError: [Errno 104] Connection reset by peer
----------------------------------------

But, from an user point of view, nothing happens, everything seems correct in the browser. Also, it's very seldom, editing the same channel a few minutes later does not raise this error.

I have checked the original url being INFO:django.server:"GET /channels/05436ac33a2d41e1ac600dfdf0e14835/ HTTP/1.1" 200 111499
and the browser network tab shows
image
so it seems the application self-recovers from the problem

@rtibbles rtibbles force-pushed the django2.2 branch 2 times, most recently from 7f9942d to 634637d Compare December 15, 2020 20:40
@rtibbles rtibbles changed the title Upgrade to django 2.2 Upgrade to django 3.1 Dec 15, 2020
@rtibbles rtibbles force-pushed the django2.2 branch 3 times, most recently from 5e9051d to d26a854 Compare December 16, 2020 23:50
Copy link
Copy Markdown
Member

@jredrejo jredrejo left a comment

Choose a reason for hiding this comment

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

Everything works perfectly after the requirements*txt files are regenerated (current ones have some minor problems with python 3.8, but with pip-compile requirements.in it's solved) . This is not a fault from this PR but from the https://github.com/rtibbles/studio/blob/django2.2/README.md file , it should say it before executing the pip install -r requirements.txt command.
In that case we could change this README.md to say that this works with Python 3.6-3.8

rtibbles added 2 commits May 19, 2021 12:28
Delete deployed squashed migrations.
Remove tests for removed migrations.
Remove some deprecated calls.
@rtibbles rtibbles marked this pull request as ready for review May 19, 2021 23:15
@rtibbles rtibbles merged commit 292b211 into learningequality:unstable May 20, 2021
@rtibbles rtibbles deleted the django2.2 branch May 20, 2021 00:09
@bjester bjester changed the title Upgrade to django 3.1 Upgrade to django 3.2 May 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants