From b20ac9c5154fdf591c0088c34d61b8f52347910d Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Tue, 2 Apr 2024 14:43:52 -0400 Subject: [PATCH 1/6] fix: Be able to clear the process_cache manually in Python 3.11 Given code like the following ``` class Foo: @process_cached def bar(self): pass ``` In Python 3.8 referencing `bar` would not call its `__get__` method. ``` x = Foo().bar ``` However in Python 3.11, making the same call would call the `__get__` method, permanently replacing the underlying `process_cached` object with the partial function that references it. This meant that code to clear the cache would work in Python 3.8 but would break in 3.11 ``` Foo().bar.cache.clear() # Works in 3.8 but not in 3.11 ``` In 3.11 this results in the following error: ``` E AttributeError: 'functools.partial' object has no attribute 'cache' ``` To make this compatible in both version, we just add the cache as an accessible attribute on the partial we generate for our wrapped function. --- openedx/core/lib/cache_utils.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/openedx/core/lib/cache_utils.py b/openedx/core/lib/cache_utils.py index 1b3b65f378a4..b53aeedf884f 100644 --- a/openedx/core/lib/cache_utils.py +++ b/openedx/core/lib/cache_utils.py @@ -147,7 +147,10 @@ def __get__(self, obj, objtype): """ Support instance methods. """ - return functools.partial(self.__call__, obj) + partial = functools.partial(self.__call__, obj) + # Make the cache accessible on the wrapped object so it can be cleared if needed. + partial.cache = self.cache + return partial class CacheInvalidationManager: From 87b9c759f0c52f27e8db968ffe952a4bf1f5f670 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Fri, 15 Mar 2024 15:30:01 -0400 Subject: [PATCH 2/6] fix: Provide a sequence to random.sample The sample function used to automatically convert sets to sequences but that is no longer supported starting in 3.11 so we have to do it manually. Reference: https://docs.python.org/3/library/random.html#random.sample --- xmodule/library_content_block.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xmodule/library_content_block.py b/xmodule/library_content_block.py index 4edcca5f00b6..6c8965186d51 100644 --- a/xmodule/library_content_block.py +++ b/xmodule/library_content_block.py @@ -221,7 +221,7 @@ def make_selection(cls, selected, children, max_count, mode): overlimit_block_keys = set() if len(selected_keys) > max_count: num_to_remove = len(selected_keys) - max_count - overlimit_block_keys = set(rand.sample(selected_keys, num_to_remove)) + overlimit_block_keys = set(rand.sample(list(selected_keys), num_to_remove)) selected_keys -= overlimit_block_keys # Do we have enough blocks now? @@ -233,7 +233,7 @@ def make_selection(cls, selected, children, max_count, mode): pool = valid_block_keys - selected_keys if mode == "random": num_to_add = min(len(pool), num_to_add) - added_block_keys = set(rand.sample(pool, num_to_add)) + added_block_keys = set(rand.sample(list(pool), num_to_add)) # We now have the correct n random children to show for this user. else: raise NotImplementedError("Unsupported mode.") From 08b3f0bf32c9a001c548b7612edbe90e10e4e21c Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Fri, 15 Mar 2024 15:16:18 -0400 Subject: [PATCH 3/6] fix: Create a bad unicode file differently. In Python 3.11 CSV files are allowed to have null characters so the test data is no longer a valid. We update it to not have a valid unicode character to still test this code path correctly. I'm not concerned about the fact that the files with null will get past this test beacause there are other checks on the content of the file that catch if it doesn't have enough or the right fields so this should be a safe change to make to the tests. Relevant Change in Python: https://github.com/python/cpython/issues/71767 --- lms/djangoapps/instructor/tests/test_api.py | 2 +- lms/djangoapps/instructor/tests/test_certificates.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 00b0fc56b05f..870494900f89 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -713,7 +713,7 @@ def test_bad_file_upload_type(self): """ Try uploading some non-CSV file and verify that it is rejected """ - uploaded_file = SimpleUploadedFile("temp.csv", io.BytesIO(b"some initial binary data: \x00\x01").read()) + uploaded_file = SimpleUploadedFile("temp.csv", io.BytesIO(b"some initial binary data: \xC3\x01").read()) response = self.client.post(self.url, {'students_list': uploaded_file}) assert response.status_code == 200 data = json.loads(response.content.decode('utf-8')) diff --git a/lms/djangoapps/instructor/tests/test_certificates.py b/lms/djangoapps/instructor/tests/test_certificates.py index 9921ae6d6831..ce3433f312d8 100644 --- a/lms/djangoapps/instructor/tests/test_certificates.py +++ b/lms/djangoapps/instructor/tests/test_certificates.py @@ -888,7 +888,7 @@ def test_bad_file_upload_type(self): """ Try uploading CSV file with invalid binary data and verify that it is rejected """ - uploaded_file = SimpleUploadedFile("temp.csv", io.BytesIO(b"some initial binary data: \x00\x01").read()) + uploaded_file = SimpleUploadedFile("temp.csv", io.BytesIO(b"some initial binary data: \xC3\x01").read()) response = self.client.post(self.url, {'students_list': uploaded_file}) assert response.status_code == 200 data = json.loads(response.content.decode('utf-8')) From 884fe8ace9f3808cddbf6f262a63fa41c2d02acf Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Fri, 15 Mar 2024 13:01:57 -0400 Subject: [PATCH 4/6] fix: Fix function mocking. The way the patch decorator was being used is not supported in python 3.11. Use the patch decorator to auto generate the correct mock and make the test a bit more readabale. The new change is both 3.8 and 3.11 compatible. --- lms/djangoapps/instructor/tests/test_api.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 870494900f89..0cb7e009454e 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -920,15 +920,16 @@ def test_users_created_and_enrolled_successfully_if_others_fail(self): manual_enrollments = ManualEnrollmentAudit.objects.all() assert manual_enrollments.count() == 2 - @patch('lms.djangoapps.instructor.views.api', 'generate_random_string', - Mock(side_effect=['first', 'first', 'second'])) def test_generate_unique_password_no_reuse(self): """ generate_unique_password should generate a unique password string that hasn't been generated before. """ - generated_password = ['first'] - password = generate_unique_password(generated_password, 12) - assert password != 'first' + with patch('lms.djangoapps.instructor.views.api.generate_random_string') as mock: + mock.side_effect = ['first', 'first', 'second'] + + generated_password = ['first'] + password = generate_unique_password(generated_password, 12) + assert password != 'first' @patch.dict(settings.FEATURES, {'ALLOW_AUTOMATED_SIGNUPS': False}) def test_allow_automated_signups_flag_not_set(self): From 6ea63da96916a58e5d27f290e106c5effac80a84 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Fri, 15 Mar 2024 13:00:22 -0400 Subject: [PATCH 5/6] fix: Don't use the deprecated location for Hashable The Hashable object was moved in python 3.3 and support for the old location is dropped in python 3.10 the new location is available in python 3.8 so we can just update this and it should work with both python 3.8 and 3.11 https://docs.python.org/3.8/library/collections.html --- openedx/core/lib/cache_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openedx/core/lib/cache_utils.py b/openedx/core/lib/cache_utils.py index b53aeedf884f..c379ab2d961a 100644 --- a/openedx/core/lib/cache_utils.py +++ b/openedx/core/lib/cache_utils.py @@ -126,7 +126,7 @@ def __init__(self, func): self.cache = {} def __call__(self, *args): - if not isinstance(args, collections.Hashable): + if not isinstance(args, collections.abc.Hashable): # uncacheable. a list, for instance. # better to not cache than blow up. return self.func(*args) From 6fb59639af395156ec362ba00dcfcb24293b6120 Mon Sep 17 00:00:00 2001 From: Feanil Patel Date: Fri, 15 Mar 2024 12:29:26 -0400 Subject: [PATCH 6/6] fix: Remove deprecated getargspec call. This function was removed by python 3.11 so update to the alternate call that is the current recommended replacement. https://docs.python.org/3.11/library/inspect.html#inspect.getfullargspec --- xmodule/tests/xml/factories.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xmodule/tests/xml/factories.py b/xmodule/tests/xml/factories.py index 23a57898b6dc..a62f7b3f49bb 100644 --- a/xmodule/tests/xml/factories.py +++ b/xmodule/tests/xml/factories.py @@ -57,7 +57,7 @@ def __repr__(self): # Extract all argument names used to construct XmlImportData objects, # so that the factory doesn't treat them as XML attributes -XML_IMPORT_ARGS = inspect.getargspec(XmlImportData.__init__).args # lint-amnesty, pylint: disable=deprecated-method +XML_IMPORT_ARGS = inspect.getfullargspec(XmlImportData.__init__).args class XmlImportFactory(Factory):