Update requirements to support Django 2.2

[kaizoku]

This updates problem-builder to support Django 2.2.

JIRA tickets: Implements BB-2061.

Testing instructions:

1. Follow XBlock workbench installation instructions
2. Check example problems in workbench.
3. Run integration and quality tests.

Author notes and concerns:

1. There are some exceptions when running integration tests, these also exist in the Django 1.11 version, but are all related to undefined variables in the template's context which are only shown in DEBUG mode or in tests.

Reviewers

• (@lgp171188)

[lgp171188]

@kaizoku, the version of Django in test_requirements.txt hasn't been updated to the latest 2.2.x version. Can you check and update that as well?

[kaizoku]

I can't get pip-compile to run successfully after updating this requirement in test_requirements.in as it includes base and test requirements from the xblock-sdk, which doesn't support Django 2.2 yet. Should we wait for https://github.com/edx/xblock-sdk to update?

[lgp171188]

@kaizoku, do you know which requirements in the xblock-sdk repo are causing this issue? If we are using the 0.1.7 release and that has this issue, can you check if this has been fixed in the master branch after the release? If yes, we can request edX to tag a new release so that we can use it. Otherwise, we should get in touch with edX about this and report this issue as the https://github.com/edx/xblock-sdk/pull/165 PR was for adding Django 2.2 support.

[kaizoku]

@lgp171188 I was testing with master before, but their requirements/base.txt which problem-builder inherits specifies django==1.11.26 in the PR, and has the same requirement in the current master.

requirements/base.in specifies Django>=1.11, but pip-compile sets that to ==1.11.26 for some reason.

[kaizoku]

I managed to get this working, but I have to upgrade xblock-sdk/requirements/base.in and the requirement for fs-s3fs to 0.1.8 in order to fix the issues with resolving six dependencies.

Should I create a PR for xblock-sdk to update that as well?

[lgp171188]

@kaizoku, yes, please go ahead and create the xblock-sdk PR. If we can find the right person in edX to ping for this, we can get it reviewed and merged soon. @bradenmacdonald can you tell us who from edX requested us to update problem-builder to support Django 2.2?

[lgp171188]

@kaizoku, I tested the changes in this PR in the workbench and it works okay. Please check my review comment and update the PR to address it.

[jmbowman]

Sorry, I lost track of the related xblock-sdk PR. It's now been merged, so you should be clear to continue working on this. We're starting to run the edx-platform test suite under Django 2.x now, all known deprecation warnings and the vast majority of the outdated dependencies have been dealt with.

[jmbowman]

Any updates on this? We're in the final stretch of fixing edx-platform tests under Django 2.x, and official support for Django 1.11 could end as early as tomorrow.

[kaizoku]

@jmbowman yep, working on getting the couple failing tests to pass.

Maybe it's due to my unfamiliarity with translations, but there's something about xblock-utils modifying the trans tag causing this issue: https://github.com/edx/xblock-utils/blob/88891cd4a494f4561fb609c4b600cf6d2a9733b4/xblockutils/templatetags/i18n.py#L29

It's returning a django.utils.functional.lazy proxy, which is failing to turn into a string.
I see some changes from Django 1.12 to 2.2 in the translation functions for ugettext and the like, which could be the trouble here, but I haven't had time to get into them and see if that's the real issue here. @lgp171188 do you have any ideas on this? Are you well versed with the django translation utilities?

[lgp171188]

@lgp171188 do you have any ideas on this? Are you well versed with the django translation utilities?

@kaizoku, I am not familiar with this issue, but will take a look tomorrow. Btw, as discussed on the task, we should create a new major version release branch and target this PR for that branch.

[jmbowman]

Another update: all edx-platform tests are now passing under Django 2.2 (they don't install this XBlock), and we're finalizing the deployment plan. We're going to need this really soon, do you need help with it?

[kaizoku]

@lgp171188 Should we update to problem-builder-v4 ?
I can create the branch and change the PR base if that sounds good.

I narrowed down that the issue is due to how we're patching the xblock runtime in tests.
It's failing when we patch like so:

problem-builder/problem_builder/tests/unit/test_problem_builder.py

Line 57 in 695ab49

with patch.object(block, 'runtime') as patched_runtime:

For those tests, gettext() returns a MagicMock object, rather than just a string, which is causing the issues. In one of the tests, we patch runtime.service().gettext, but I don't think that's the gettext which is called.

I'm out of time for today, but will get back to this tomorrow.

Sorry for the delay @jmbowman. If you do have an idea here it would certainly help, but this is close to ready. We just have to fix these last couple unit tests.

[lgp171188]

@lgp171188 Should we update to problem-builder-v4 ?
I can create the branch and change the PR base if that sounds good.

@kaizoku, that sounds good to me since we have decided to create a new major version to indicate backwards incompatibility due to dropping Python 2 support.

[jmbowman]

This change fixes the tests locally for me:

--- a/problem_builder/tests/unit/test_problem_builder.py
+++ b/problem_builder/tests/unit/test_problem_builder.py
@@ -4,6 +4,7 @@ from random import random
 import ddt
 from unittest.mock import MagicMock, Mock, PropertyMock, patch
 from xblock.field_data import DictFieldData
+from xblock.runtime import NullI18nService
 
 from problem_builder.answer import AnswerRecapBlock
 from problem_builder.mcq import MCQBlock
@@ -56,6 +57,7 @@ class TestMentoringBlock(BlockWithChildrenTestMixin, unittest.TestCase):
 
         with patch.object(block, 'runtime') as patched_runtime:
             patched_runtime.publish = Mock()
+            patched_runtime.service = lambda _, service: NullI18nService() if service == 'i18n' else MagicMock()
 
             block.student_view(context={})
 
@@ -68,6 +70,7 @@ class TestMentoringBlock(BlockWithChildrenTestMixin, unittest.TestCase):
 
         with patch.object(block, 'runtime') as patched_runtime:
             patched_runtime.publish = Mock()
+            patched_runtime.service = lambda _, service: NullI18nService() if service == 'i18n' else MagicMock()
 
             block.student_view(context={})
 
@@ -95,7 +98,7 @@ class TestMentoringBlock(BlockWithChildrenTestMixin, unittest.TestCase):
 
         with patch.object(block, 'runtime') as patched_runtime:
             patched_runtime.publish = Mock()
-            patched_runtime.service().ugettext = lambda str: str
+            patched_runtime.service = lambda _, service: NullI18nService() if service == 'i18n' else MagicMock()
             patched_runtime.get_block = lambda block_id: None
             patched_runtime.load_block_type = lambda block_id: Mock

[bradenmacdonald]

@kaizoku The tests aren't flaky, there seem to be genuine bugs preventing them from running. Were you able to run them locally?

Since this is blocking edX, I'm OK with merging it now if we fix the integration tests before closing this ticket, and if we have manually verified that problem builder and step builder are working in Studio + LMS with Django 2.2.

[jmbowman]

This is now officially the only blocker for deploying edx-platform on Django 2.2, and all the other edx.org services are either already on it or being deployed today. Which makes this also the current blocker for cutting a Juniper release. Anything we can do to help get this over the line?

[lgp171188]

@jmbowman, sorry for the delay! We've been working on getting this done asap this since your ping earlier this week. We've had to fix various issues and are down to the last failing integration test. We will be able to merge this PR and tag a new release if we can either fix it or confirm that it is flaky.

We have also done a lot of manual testing with this PR on the master devstack and found nothing broken. But the CI checks passing in this PR will be a good confirmation of everything working okay.

[lgp171188]

@jmbowman, we noticed that there is now a fork of django-babel-underscore under the edx organization which also has the changes to support Python 3, Django 2.2 merged. Is it possible to tag a new release so that problem-builder can use that? If not, we can use the current HEAD of the master branch if that's okay.

[jmbowman]

Here's what we're using for django-babel-underscore and enmerkar in edx-platform: https://github.com/edx/edx-platform/blob/master/requirements/edx/github.in#L64-L68

[lgp171188]

Here's what we're using for django-babel-underscore and enmerkar in edx-platform: https://github.com/edx/edx-platform/blob/master/requirements/edx/github.in#L64-L68

@jmbowman, that's exactly the same commit of edx/django-babel-underscore that we are using. We have dropped the original author's repo and the older version because this change will be merged to the 4.0.0 branch in which Django versions <2.2 are not supported.

It looks like we have managed to fix all the CI issues and the current run should likely succeed. Once that is done, I will merge the PR, tag a new release and ping you.

[lgp171188]

👍

• I tested this in the workbench and on the master devstack and everything is working fine.
• I read through the code
• I checked for accessibility issues NA
• Includes documentation

[lgp171188]

@jmbowman, I have merged this PR to the problem-builder-v4 branch and tagged a new 4.0.0 release.

[jmbowman]

Thanks, created the configuration and edx-internal PRs to upgrade it, now creating the edx-platform Django 2.2 upgrade PR.