Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Post Django 3.2 cleanup #151

Merged
merged 1 commit into from
Feb 23, 2022

Conversation

edx-requirements-bot
Copy link
Contributor

@edx-requirements-bot edx-requirements-bot commented Feb 10, 2022

JIRA: BOM-3181

DESC

This PR is a part of Django32 cleanup effort which aims at dropping outdated Django versions and also adding Django40 support.

Additional information from script execution

Python code cleanup by the cleanup-python-code Jenkins job.

This pull request was generated by the cleanup-python-code Jenkins job, which ran
modernize_github_actions_django; modernize_github_actions; modernize_tox; modernize_setup_file; djcodemod run --removed-in 4.0 .;sed -i -E 's/django(<|>)[0-9]*\.[0-9]+/django<4.0/gi' ./requirements/constraints.txt; make upgrade;

The following packages were installed:
edx-repo-tools django-codemod

Deleted obsolete pull_requests:
#148

@edx-requirements-bot edx-requirements-bot requested review from mraarif and a team February 10, 2022 13:02
@aht007 aht007 force-pushed the jenkins/cleanup-python-code-6ecab53 branch 3 times, most recently from 4a4e950 to ef26096 Compare February 16, 2022 12:56
@aht007 aht007 marked this pull request as ready for review February 16, 2022 12:58
@@ -1,9 +1,20 @@
amqp==5.0.9
#
Copy link
Member

Choose a reason for hiding this comment

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

This file was supposed to have celery44 dependencies in it but in the last Upgrade Job these were updated mistakenly as they were not constrained. I have done a workaround to keep these requirements constrained and also made necessary changes for celery50 requirements.

@aht007 aht007 force-pushed the jenkins/cleanup-python-code-6ecab53 branch from ef26096 to 58a45a6 Compare February 16, 2022 13:53
# Let tox control the Django version for tests
grep -e "^amqp==\|^anyjson==\|^billiard==\|^celery==\|^kombu==\|^click-didyoumean==\|^click-repl==\|^click==\|^prompt-toolkit==\|^vine==" requirements/base.txt > requirements/celery44.txt
grep -e "^amqp==\|^anyjson==\|^billiard==\|^celery==\|^kombu==\|^click-didyoumean==\|^click-repl==\|^click==\|^prompt-toolkit==\|^vine==" requirements/base.txt > requirements/celery52.txt
Copy link
Contributor

Choose a reason for hiding this comment

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

so, we don't want to maintain a celery44 dependencies list?

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we should've added a file for celery50 instead of replacing celery44 with celery50

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
grep -e "^amqp==\|^anyjson==\|^billiard==\|^celery==\|^kombu==\|^click-didyoumean==\|^click-repl==\|^click==\|^prompt-toolkit==\|^vine==" requirements/base.txt > requirements/celery52.txt
grep -e "^amqp==\|^anyjson==\|^billiard==\|^celery==\|^kombu==\|^click-didyoumean==\|^click-repl==\|^click==\|^prompt-toolkit==\|^vine==" requirements/base.txt > requirements/celery44.txt
grep -e "^amqp==\|^anyjson==\|^billiard==\|^celery==\|^kombu==\|^click-didyoumean==\|^click-repl==\|^click==\|^prompt-toolkit==\|^vine==" requirements/base.txt > requirements/celery52.txt

Copy link
Member

Choose a reason for hiding this comment

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

Latest requirements will only contain packages according to celery52 so adding the suggested step will only create duplicate celery44.txt and celery52.txt files.
In the make upgrade step, we can only fetch the requirements for the latest supported version so the requirements for celery44.txt are being generated with celery44.in now.

@UsamaSadiq UsamaSadiq force-pushed the jenkins/cleanup-python-code-6ecab53 branch from 58a45a6 to 8dc06a0 Compare February 22, 2022 13:23
@UsamaSadiq UsamaSadiq force-pushed the jenkins/cleanup-python-code-6ecab53 branch from ea01048 to 50f1297 Compare February 23, 2022 09:37
@UsamaSadiq UsamaSadiq merged commit c6fdba6 into master Feb 23, 2022
@UsamaSadiq UsamaSadiq deleted the jenkins/cleanup-python-code-6ecab53 branch February 23, 2022 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants