Update Django to 3.2, remove or update package constraints by symbolist · Pull Request #128 · openedx-unsupported/blockstore

symbolist · 2021-09-29T13:59:35Z

Description

Upgrades to the latest LTS version of Django
Some other unneeded constraints (which had been set a while ago) have also been removed.
Removes the corheaders app + set up.

Ticket

Author Comments, Concerns, and Open Questions

None.

Test Instructions

Test you are able to create, update and delete atomic and composite XBlocks via the LX devstack.
Test file uploads to bundles work as well.

openedx-webhooks · 2021-09-29T13:59:40Z

Thanks for the pull request, @symbolist! I've created OSPR-6082 to keep track of it in JIRA.

As a core committer in this repo, you can merge this once the pull request is approved per the core committer reviewer requirements and according to the agreement with your edX Champion.

natabene · 2021-09-29T20:10:56Z

@symbolist Thank you for your contribution. Would @bradenmacdonald be interested in reviewing?

bradenmacdonald · 2021-09-29T22:46:06Z

@natabene @symbolist Sure, I can review - just let me know when it's ready.

…constraint.

symbolist · 2021-10-05T19:41:19Z

I tried running this on the LX CI and there were a few test failures. They did seem unrelated but still investigating them.

symbolist

@bradenmacdonald This is ready for review (see also comment on ticket).

CC @natabene

symbolist · 2021-10-06T13:32:38Z

Dockerfile-3.8

 python3-pip \
- python3-venv
+ python3-venv \
+ wget


Needed for the make upgrade command.

symbolist · 2021-10-06T13:33:08Z

Makefile

 upgrade: export CUSTOM_COMPILE_COMMAND=make upgrade
 upgrade: $(COMMON_CONSTRAINTS_TXT)	## update the requirements/*.txt files with the latest packages satisfying requirements/*.in
+	sed 's/Django<2.3//g' requirements/common_constraints.txt > requirements/common_constraints.tmp
+	mv requirements/common_constraints.tmp requirements/common_constraints.txt


Need to remove this line to update to Django 3.2 which is specified in constraints.txt.

symbolist · 2021-10-06T13:34:07Z

blockstore/apps/api/routers.py

-                ) for (lookup_url_kwarg, lookup_value_regex) in zip(lookup_url_kwargs, lookup_value_regexes)
+                f'(?P<{lookup_prefix}{lookup_url_kwarg}>{lookup_value_regex})' for (
+                    lookup_url_kwarg, lookup_value_regex
+                ) in zip(lookup_url_kwargs, lookup_value_regexes)


There are a few conversions to f-strings because of quality issues raised by pylint version upgrade.

symbolist · 2021-10-06T13:34:59Z

blockstore/apps/bundles/storage.py

-    def get_alternative_name(self, name):
-        return default_storage.get_alternative_name(name)
+    def get_alternative_name(self, file_root, file_ext):
+        return default_storage.get_alternative_name(file_root, file_ext)


A parameter was added in Django 3. Though this method does not seem to be in use in this package.

symbolist · 2021-10-06T13:37:26Z

blockstore/apps/core/migrations/0003_alter_user_first_name.py

+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):


Added by upgrade to Django 3.2. The only change seems to be the change in max_length.

symbolist · 2021-10-06T13:40:09Z

requirements/base.in

 django
-django-cors-headers
-django-environ-2
+django-environ


Switched back to django-environ.

This was earlier changed in #121.

symbolist · 2021-10-06T13:41:20Z

requirements/constraints.txt

-# Not yet using Django 3.X. Using the 2.X LTS to mirror the LMS.
-django>2.2,<2.3
-
-django-cors-headers>=2.2.0,<2.3


Not needed so has been removed.

symbolist · 2021-10-06T13:45:18Z

requirements/constraints.txt

-django-environ-2==2.1.0
-
-django-filter==2.1.0
+django>3.2,<3.3


There don't seem to be any backwards incompatible changes relevant to this repo:

https://docs.djangoproject.com/en/3.2/releases/3.0/#backwards-incompatible-changes-in-3-0

symbolist · 2021-10-06T13:48:53Z

requirements/base.txt

-    # via
-    #   -c requirements/constraints.txt
-    #   -r requirements/base.in
+django-environ==0.7.0


This was originally pinned to v.0.4.5

There don't seem to be any backwards incompatible changes between 0.4.5 and 0.7.0: https://github.com/joke2k/django-environ/blob/main/CHANGELOG.rst

symbolist · 2021-10-06T13:50:34Z

requirements/base.txt

-    #   -r requirements/base.in
+django-environ==0.7.0
+    # via -r requirements/base.in
+django-filter==21.1


Constraint on this has been removed. Though there do not seem to be any backwards incompatible changes since 2.1.0: https://github.com/carltongibson/django-filter/blob/main/CHANGES.rst

symbolist · 2021-10-06T13:56:57Z

CC @kdmccormick Just a heads up that this will be coming your way for a deploy. 🙂

bradenmacdonald

👍 Looks good to me. Thanks!

I did encounter one issue with make easyserver, but it seems unrelated to this PR: the default docker-compose project name is now blockstore3.8 but docker sometimes interprets it as blockstore3 (ignoring parts after the .) which causes enough consistency errors to stop everything from working. I could only get the server running by using BLOCKSTORE_PROJECT_NAME=blockstore38 make easyserver

I tested this: some basic read+write tests using frontend-app-library-authoring, also confirmed LX test suite is passing
I read through the code
I checked for accessibility issues: n/a
Includes documentation

openedx-webhooks · 2021-10-08T00:42:01Z

@ormsbee, @kdmccormick: thought you might like to know that symbolist merged this pull request.

openedx-webhooks · 2021-10-08T00:42:01Z

@symbolist 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

Using a period in the docker-compose project name has been observed to cause issues. For example: openedx-unsupported#128 (review) So, normalize all references to 3.8 to 38. This fixes the issue, and still retains the filenaming linked with the python version used.

Using a period in the docker-compose project name has been observed to cause issues when using docker-compose 2.0. For example: openedx-unsupported#128 (review) So, normalize all references to 3.8 to 38. This fixes the issue, and still retains the filenaming linked with the python version used.

openedx-webhooks added core committer open-source-contribution PR author is not from Axim or 2U waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Sep 29, 2021

symbolist mentioned this pull request Sep 30, 2021

Remove tagstore. #96

Merged

symbolist force-pushed the symbolist/update-versions branch 7 times, most recently from 06bb859 to 0da0788 Compare October 5, 2021 12:19

symbolist added 5 commits October 5, 2021 18:15

build: update dependencies.

ca737cf

build: Add wget. It is needed by make upgrade.

a5607dc

build: Switch back to the original django-environ and remove version …

1056baa

…constraint.

build: Remove constraint on django-filter package.

253dc44

feat!: Remove cors config since it is not being used.

eeaab47

symbolist force-pushed the symbolist/update-versions branch from 0da0788 to 18d15d7 Compare October 5, 2021 13:53

build: Update Django to version 3.2.

a62a33e

symbolist force-pushed the symbolist/update-versions branch from 18d15d7 to a62a33e Compare October 5, 2021 18:00

symbolist changed the title ~~Remove or update package constraints~~ Update Django to 3.2, remove or update package constraints Oct 6, 2021

symbolist commented Oct 6, 2021

View reviewed changes

bradenmacdonald approved these changes Oct 8, 2021

View reviewed changes

bradenmacdonald merged commit 95de3d2 into master Oct 8, 2021

bradenmacdonald deleted the symbolist/update-versions branch October 8, 2021 00:41

openedx-webhooks removed the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Oct 8, 2021

openedx-webhooks added the merged label Oct 8, 2021

bradenmacdonald mentioned this pull request Oct 14, 2021

Revert back to django-environ #134

Closed

BbrSofiane mentioned this pull request Oct 19, 2021

Update or replace django-environ edx/upgrades#21

Closed

cmltaWt0 mentioned this pull request Oct 28, 2021

chore: upgrade django-dynamic-fixture version #130

Closed

samuelallan72 mentioned this pull request Oct 29, 2021

FAL-2522 Remove period from python version makefile var #136

Merged

UsamaSadiq mentioned this pull request Jan 17, 2022

Update or replace django-dotenv edx/upgrades#22

Open

		from django.db import migrations, models


		class Migration(migrations.Migration):

Conversation

symbolist commented Sep 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Ticket

Author Comments, Concerns, and Open Questions

Test Instructions

Uh oh!

openedx-webhooks commented Sep 29, 2021

Uh oh!

natabene commented Sep 29, 2021

Uh oh!

bradenmacdonald commented Sep 29, 2021

Uh oh!

symbolist commented Oct 5, 2021

Uh oh!

symbolist left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

symbolist commented Oct 6, 2021

Uh oh!

bradenmacdonald left a comment

Choose a reason for hiding this comment

Uh oh!

openedx-webhooks commented Oct 8, 2021

Uh oh!

openedx-webhooks commented Oct 8, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

symbolist commented Sep 29, 2021 •

edited

Loading