build: add python 3.11 and 3.12 ci checks #4153

Danyal-Faheem · 2024-04-18T09:22:29Z

⛔️ MAIN BRANCH WARNING! 2U EMPLOYEES must make branches against the 2u/main BRANCH

I have checked the branch to which I would like to merge.

⛔️ DEPRECATION WARNING

Anyone internally merging to this repository is expected to release and monitor their changes; if you are not able to do this DO NOT MERGE, please coordinate with someone who can to ensure that the changes are released.

Required Testing

Before deploying this change, complete a purchase in the stage environment.
(^ We can remove that manual check once REV-2624 is done and the corresponding e2e test runs again)

Description

Added CI checks as part of the upgrade process to python 3.11 and python 3.12.
Also fixes one part of the downstream issue in tutor-ecommerce.
Updated docker-compose to docker compose.
Install python3.x in the container and its dependencies for testing.
Run tox as python3.x -m tox to make sure tox is run in the specific python environment.
Added the PYTHON_ENV and PYTHON_VERSION variables to utilize the matrix values to test against multiple python versions.
Updated the tox.ini fle to include py311 and py312 as well.
Build docs against python 3.11 and 3.12 as well.
Updated requirements and removed certain constraints to make them compatible with py311 and py312.
Run migrations on pending models.
Update pylintrc with new warnings to ignore after updating pylint version.

DawoudSheraz · 2024-04-29T11:33:22Z

@feanil Hi. Can you run the workflows on this PR, please? Thanks

andrey-canon

Hi @Danyal-Faheem, firstly, I would like to ask to fix the automatic tests and secondly, these changes are not doing what is supposed to do since this is triggering 3 different actions but all of them are running with python 3.8 indeed the 3.12 test shouldn't pass since that depends on #4147

Danyal-Faheem · 2024-05-03T06:03:46Z

Hi @andrey-canon. Thank you for pointing out the problem. The Dockerfile is currently forcing python3.8 in the containers. I'll update the PR to allow different python versions to be installed.

feanil · 2024-05-09T16:21:55Z

@Danyal-Faheem do we have an update on this PR? It looks like once we get it working, we'll need to backport this to Redwood as well.

Danyal-Faheem · 2024-05-10T06:47:22Z

Hi @feanil, we're still working on this PR. It needs some changes in the Dockerfile as well. We will try to get it done as soon as possible so that we can backport it to redwood.

Faraz32123 · 2024-05-28T10:48:33Z

@Danyal-Faheem and I have tested the changes, there's still some python tests failing under test-python workflow.
We have added pylintrc file to ignore some of the pylint warnings. We also had to upgrade some requirements so that CI checks also work with python 3.(8,11,12).
Thats the update for now!

pin django-oscar to 3.2, removed migrations, replace depreciated assertDictContainsSubset method with python code

Faraz32123 · 2024-05-31T09:06:34Z

Hi @feanil, this PR is ready for review. Can you take a look? Thanks.

DawoudSheraz · 2024-05-31T09:43:58Z

pylintrc_backup

@@ -0,0 +1,294 @@
+[MASTER]


nit: this file wont be needed.

DawoudSheraz · 2024-05-31T09:44:31Z

requirements/base.in

@@ -31,6 +31,7 @@ edx-django-utils
 edx-drf-extensions>=8.13.0  # 8.13 fixes forgiven JWTs for ecommerce
 edx-django-sites-extensions
 edx-ecommerce-worker
+edx-lint


rather than base, this should be in test.in as this is only needed during testing.

DawoudSheraz · 2024-05-31T10:00:14Z

ecommerce/credit/tests/test_views.py

@@ -130,7 +130,18 @@ def _assert_success_checkout_page(self, sku=None):

        response = self.client.get(self.path)
        self.assertEqual(response.status_code, 200)
-        self.assertDictContainsSubset({'course': self.course}, response.context)
+        if sys.version_info > (3, 9):
+            context = {}


A comment here to explain why this is needed would be helpful. Plus, see if this can be improved. There are a bit too many if-elses in the test.

adding a comment for now.

Will also look for the alternative for the if-else.

DawoudSheraz · 2024-05-31T10:01:19Z

requirements/dev.in

@@ -2,7 +2,7 @@
 -r docs.txt

 django-debug-toolbar
-
+needle


Why is this needed?

Its not longer needed, It was added to resolve conlicts. removing it.

DawoudSheraz · 2024-05-31T10:02:02Z

requirements/dev.in

@@ -12,3 +12,5 @@ ptvsd

 # For devserver code reloading
 pywatchman
+astroid==3.2.2


Why is astroid needed here? Furthermore, since the version has been constrained in constraints.txt, it wont be needed here.

astroid had a conflict with pylint version. So, it needs to be pinned.
I'll remove it from dev.in as it is in contraints.txt.

DawoudSheraz · 2024-05-31T10:03:21Z

requirements/dev.in

@@ -12,3 +12,5 @@ ptvsd

 # For devserver code reloading
 pywatchman
+astroid==3.2.2
+edx-lint


it won't be required here. Once it is added to test.in, it should start appearing in dev.txt

Faraz32123 · 2024-06-05T09:23:34Z

Hi @feanil, this PR is ready for review. Can you take a look? Thanks.

Hello @feanil, Hope so you are doing well. A soft reminder for you. So that this issue can be closed. Thanks.

Faraz32123 · 2024-06-05T09:38:22Z

Hello @andrey-canon,
Can u also have a look at this PR for me. Thanks.

andrey-canon · 2024-06-05T15:52:25Z

Hello @andrey-canon, Can u also have a look at this PR for me. Thanks.

Hi @Faraz32123, sure, no problem, however I cannot do that this week, so I will review this next one : D

Faraz32123 · 2024-06-11T07:01:33Z

Hello @feanil and @andrey-canon, I hope you guys are doing well, a reminder for you guys to review this PR for this issue to be closed. Thanks.

andrey-canon

Most of the changes look good to me. I just left a suggestion related to the test changes because, personally, I don't see the point of implementing two different ways for different Python versions when one of them is compatible with both (I think I didn't test that). If so could you please apply that to all the tests modifications

andrey-canon · 2024-06-12T13:59:19Z

ecommerce/credit/tests/test_views.py

@@ -130,7 +130,21 @@ def _assert_success_checkout_page(self, sku=None):

        response = self.client.get(self.path)
        self.assertEqual(response.status_code, 200)
-        self.assertDictContainsSubset({'course': self.course}, response.context)
+        if sys.version_info > (3, 9):
+            # assertDictContainsSubset is depreciated in python version>3.9


Suggested change

# assertDictContainsSubset is depreciated in python version>3.9

# assertDictContainsSubset is deprecated in Python version > 3.9

???

andrey-canon · 2024-06-12T14:01:08Z

ecommerce/credit/tests/test_views.py

-        self.assertDictContainsSubset({'course': self.course}, response.context)
+        if sys.version_info > (3, 9):
+            # assertDictContainsSubset is depreciated in python version>3.9
+            # context.response return ContextList object, belwo statements will convert it to dict


Suggested change

# context.response return ContextList object, belwo statements will convert it to dict

# response.context returns a ContextList object; the below statements will convert it to a dict

??

andrey-canon · 2024-06-12T14:01:39Z

ecommerce/credit/tests/test_views.py

+        if sys.version_info > (3, 9):
+            # assertDictContainsSubset is depreciated in python version>3.9
+            # context.response return ContextList object, belwo statements will convert it to dict
+            # assertLessEqual method is used instead of depreciated assertDictContainsSubset method


Suggested change

# assertLessEqual method is used instead of depreciated assertDictContainsSubset method

# assertLessEqual method is used instead of the deprecated assertDictContainsSubset method

andrey-canon · 2024-06-12T14:42:16Z

ecommerce/core/tests/test_models.py

+        if sys.version_info > (3, 9):
+            self.assertLessEqual(expected.items(), last_request.headers.items())
+        else:
+            self.assertDictContainsSubset(expected, last_request.headers)


Suggested change

if sys.version_info > (3, 9):

self.assertLessEqual(expected.items(), last_request.headers.items())

else:

self.assertDictContainsSubset(expected, last_request.headers)

self.assertLessEqual(expected.items(), last_request.headers.items())

andrey-canon · 2024-06-12T15:55:39Z

ecommerce/enterprise/tests/test_utils.py

@@ -122,7 +123,10 @@ def test_post_enterprise_customer_user(self, mock_helpers, expected_return):
            self.learner.username
        )

-        self.assertDictContainsSubset(expected_return, response)
+        if sys.version_info > (3, 9):


same comment as above

Faraz32123 · 2024-06-13T10:21:32Z

Hello @andrey-canon, thanks for the review. I have updated the code according to the requested changes. Can u look into it again so that we can finalize it.

andrey-canon · 2024-06-13T14:23:21Z

ecommerce/core/tests/test_models.py

@@ -1,6 +1,7 @@


 import json
+import sys


I think you can remove this from all the files

andrey-canon

Thanks for applying all the suggested changes

CC @feanil LGTM

Faraz32123 · 2024-06-20T19:01:08Z

Hello @feanil,
Can you also have a look at the backport PR to redwood and merge it.
Thanks.

Danyal-Faheem requested a review from a team as a code owner April 18, 2024 09:22

andrey-canon suggested changes May 2, 2024

View reviewed changes

Danyal-Faheem force-pushed the danyalfaheem/add-python-3.11-ci-checks branch from 5955824 to 7caffb5 Compare May 3, 2024 11:23

Danyal-Faheem changed the title ~~ci: add python 3.11 and 3.12 checks~~ build: add python 3.11 and 3.12 ci checks May 3, 2024

Faraz32123 mentioned this pull request May 3, 2024

fix: update celery and libsass versions #4147

Merged

feanil linked an issue May 9, 2024 that may be closed by this pull request

[ecommerce] Test Python 3.11 and 3.12 #4161

Closed

build: add python 3.11 and python 3.12 ci checks

0190f4d

Danyal-Faheem force-pushed the danyalfaheem/add-python-3.11-ci-checks branch from 7caffb5 to 0190f4d Compare May 28, 2024 09:59

feanil requested a review from andrey-canon May 28, 2024 14:04

Faraz32123 force-pushed the danyalfaheem/add-python-3.11-ci-checks branch from 3f976dd to ce381be Compare May 31, 2024 07:57

fix: ci checks related to python tests

323c407

pin django-oscar to 3.2, removed migrations, replace depreciated assertDictContainsSubset method with python code

Faraz32123 force-pushed the danyalfaheem/add-python-3.11-ci-checks branch from ce381be to 323c407 Compare May 31, 2024 07:59

Faraz32123 mentioned this pull request May 31, 2024

build: add python 3.11 and 3.12 ci checks #4165

Merged

DawoudSheraz reviewed May 31, 2024

View reviewed changes

pylintrc_backup Outdated

@@ -0,0 +1,294 @@

[MASTER]

Copy link

Contributor

DawoudSheraz May 31, 2024

Choose a reason for hiding this comment

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

nit: this file wont be needed.

Faraz32123 reacted with thumbs up emoji

DawoudSheraz reviewed May 31, 2024

View reviewed changes

refactor: address comments

146fe13

Faraz32123 mentioned this pull request Jun 11, 2024

openedx/ecommerce and openedx/ecommerce-worker Python 3.12 upgrade overhangio/tutor-ecommerce#78

Closed

4 tasks

andrey-canon suggested changes Jun 12, 2024

View reviewed changes

Faraz32123 requested a review from andrey-canon June 13, 2024 10:19

andrey-canon reviewed Jun 13, 2024

View reviewed changes

refactor: address comments

9360ac0

Faraz32123 force-pushed the danyalfaheem/add-python-3.11-ci-checks branch from 5035417 to 9360ac0 Compare June 14, 2024 09:14

Faraz32123 requested a review from andrey-canon June 14, 2024 09:14

andrey-canon approved these changes Jun 17, 2024

View reviewed changes

feanil approved these changes Jun 20, 2024

View reviewed changes

feanil merged commit 8a9be80 into openedx:master Jun 20, 2024
20 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: add python 3.11 and 3.12 ci checks #4153

build: add python 3.11 and 3.12 ci checks #4153

Danyal-Faheem commented Apr 18, 2024 •

edited

Loading

DawoudSheraz commented Apr 29, 2024

andrey-canon left a comment

Danyal-Faheem commented May 3, 2024

feanil commented May 9, 2024

Danyal-Faheem commented May 10, 2024

Faraz32123 commented May 28, 2024

Faraz32123 commented May 31, 2024

DawoudSheraz May 31, 2024

DawoudSheraz May 31, 2024 •

edited

Loading

DawoudSheraz May 31, 2024

Faraz32123 May 31, 2024

Faraz32123 May 31, 2024

DawoudSheraz May 31, 2024

Faraz32123 May 31, 2024

DawoudSheraz May 31, 2024

Faraz32123 May 31, 2024

DawoudSheraz May 31, 2024

Faraz32123 commented Jun 5, 2024

Faraz32123 commented Jun 5, 2024

andrey-canon commented Jun 5, 2024

Faraz32123 commented Jun 11, 2024

andrey-canon left a comment

andrey-canon Jun 12, 2024

andrey-canon Jun 12, 2024

andrey-canon Jun 12, 2024

andrey-canon Jun 12, 2024

andrey-canon Jun 12, 2024

Faraz32123 commented Jun 13, 2024

andrey-canon Jun 13, 2024

andrey-canon left a comment

Faraz32123 commented Jun 20, 2024

	# assertDictContainsSubset is depreciated in python version>3.9
	# assertDictContainsSubset is deprecated in Python version > 3.9

	# context.response return ContextList object, belwo statements will convert it to dict
	# response.context returns a ContextList object; the below statements will convert it to a dict

	# assertLessEqual method is used instead of depreciated assertDictContainsSubset method
	# assertLessEqual method is used instead of the deprecated assertDictContainsSubset method

build: add python 3.11 and 3.12 ci checks #4153

build: add python 3.11 and 3.12 ci checks #4153

Conversation

Danyal-Faheem commented Apr 18, 2024 • edited Loading

⛔️ MAIN BRANCH WARNING! 2U EMPLOYEES must make branches against the 2u/main BRANCH

⛔️ DEPRECATION WARNING

Anyone internally merging to this repository is expected to release and monitor their changes; if you are not able to do this DO NOT MERGE, please coordinate with someone who can to ensure that the changes are released.

Required Testing

Description

DawoudSheraz commented Apr 29, 2024

andrey-canon left a comment

Choose a reason for hiding this comment

Danyal-Faheem commented May 3, 2024

feanil commented May 9, 2024

Danyal-Faheem commented May 10, 2024

Faraz32123 commented May 28, 2024

Faraz32123 commented May 31, 2024

Choose a reason for hiding this comment

DawoudSheraz May 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Faraz32123 commented Jun 5, 2024

Faraz32123 commented Jun 5, 2024

andrey-canon commented Jun 5, 2024

Faraz32123 commented Jun 11, 2024

andrey-canon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Faraz32123 commented Jun 13, 2024

Choose a reason for hiding this comment

andrey-canon left a comment

Choose a reason for hiding this comment

Faraz32123 commented Jun 20, 2024

Danyal-Faheem commented Apr 18, 2024 •

edited

Loading

DawoudSheraz May 31, 2024 •

edited

Loading