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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 25 additions & 21 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ on:
push:
branches: [master]
pull_request:
branches:
- '**'

jobs:
run_tests:
Expand All @@ -14,28 +12,34 @@ jobs:
strategy:
matrix:
os: [ubuntu-20.04]
python-version: ['3.8']
toxenv: [celery44-django32, celery52-django32]
python-version: ["3.8"]
toxenv:
[
celery44-django32,
celery52-django32,
celery44-django40,
celery52-django40,
]

steps:
- uses: actions/checkout@v2
- name: setup python
uses: actions/setup-python@v2
with:
python-version: ${{ matrix.python-version }}
- uses: actions/checkout@v2
- name: setup python
uses: actions/setup-python@v2
with:
python-version: ${{ matrix.python-version }}

- name: Start MongoDB
uses: supercharge/mongodb-github-action@1.6.0
with:
mongodb-version: 4.0
- name: Start MongoDB
uses: supercharge/mongodb-github-action@1.6.0
with:
mongodb-version: 4.0

- name: Install pip
run: pip install -r requirements/pip.txt
- name: Install pip
run: pip install -r requirements/pip.txt

- name: Install Dependencies
run: make test.setup install
- name: Install Dependencies
run: make test.setup install

- name: Run Tests
env:
TOXENV: ${{ matrix.toxenv }}
run: tox
- name: Run Tests
env:
TOXENV: ${{ matrix.toxenv }}
run: tox
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,9 @@ upgrade: ## update the requirements/*.txt files with the latest packages satisfy
pip-compile --upgrade -o requirements/test.txt requirements/test.in
pip-compile --upgrade -o requirements/ci.txt requirements/ci.in
pip-compile --upgrade -o requirements/dev.txt requirements/dev.in
pip-compile --upgrade -o requirements/celery44.txt requirements/celery44.in
# 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.

sed -i.tmp '/^[d|D]jango==/d' requirements/test.txt
sed -i.tmp '/^djangorestframework==/d' requirements/test.txt
sed -i.tmp '/^amqp==/d' requirements/test.txt
Expand Down
2 changes: 1 addition & 1 deletion eventtracking/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
"""A simple event tracking library"""

__version__ = '2.0.0'
__version__ = '2.1.0'
1 change: 1 addition & 0 deletions requirements/celery44.in
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
celery>=4.4,<4.5
27 changes: 19 additions & 8 deletions requirements/celery44.txt
Original file line number Diff line number Diff line change
@@ -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.

# This file is autogenerated by pip-compile with python 3.8
# To update, run:
#
# make upgrade
#
amqp==2.6.1
# via kombu
billiard==3.6.4.0
celery==5.2.3
click==8.0.4
click-didyoumean==0.3.0
click-repl==0.2.0
kombu==5.2.3
prompt-toolkit==3.0.28
vine==5.0.0
# via celery
celery==4.4.7
# via -r requirements/celery44.in
kombu==4.6.11
# via celery
pytz==2021.3
# via celery
vine==1.3.0
# via
# amqp
# celery
2 changes: 1 addition & 1 deletion requirements/celery52.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@ click==8.0.3
click-didyoumean==0.3.0
click-repl==0.2.0
kombu==5.2.3
prompt-toolkit==3.0.27
prompt-toolkit==3.0.28
vine==5.0.0
7 changes: 2 additions & 5 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@
import os
import re

from setuptools import setup
from setuptools import find_packages
from setuptools import find_packages, setup

# allow setup.py to be run from any path
os.chdir(os.path.normpath(os.path.join(os.path.abspath(__file__), os.pardir)))
Expand Down Expand Up @@ -81,10 +80,8 @@ def get_version(*file_paths):
'Development Status :: 2 - Pre-Alpha',
'Environment :: Web Environment',
'Framework :: Django',
'Framework :: Django :: 2.2',
'Framework :: Django :: 3.0',
'Framework :: Django :: 3.1',
'Framework :: Django :: 3.2',
'Framework :: Django :: 4.0',
'Intended Audience :: Developers',
'License :: OSI Approved :: GNU Affero General Public License v3 or later (AGPLv3+)',
'Operating System :: OS Independent',
Expand Down
30 changes: 16 additions & 14 deletions tox.ini
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
[tox]
envlist = py38-celery{44,52}-django{32}
envlist = py38-celery{44,52}-django{32,40}

[testenv]
setenv =
DJANGO_SETTINGS_MODULE = eventtracking.django.tests.settings
PYTHONPATH = {toxinidir}
deps =
celery44: -r{toxinidir}/requirements/celery44.txt
celery52: -r{toxinidir}/requirements/celery52.txt
-r{toxinidir}/requirements/test.txt
commands =
django32: pip install 'Django>=3.2,<3.3'
pytest --cov-report=html --cov-report term-missing --cov-branch -k 'not integration and not performance' --cov-fail-under=95 --cov=eventtracking
pytest --verbose -s -k 'integration'
pycodestyle --config=setup.cfg eventtracking setup.py
pylint --rcfile=pylintrc eventtracking setup.py
setenv =
DJANGO_SETTINGS_MODULE = eventtracking.django.tests.settings
PYTHONPATH = {toxinidir}
deps =
celery44: -r{toxinidir}/requirements/celery44.txt
celery52: -r{toxinidir}/requirements/celery52.txt
django32: Django>=3.2,<4.0
django40: Django>=4.0,<4.1
-r{toxinidir}/requirements/test.txt
commands =

pytest --cov-report=html --cov-report term-missing --cov-branch -k 'not integration and not performance' --cov-fail-under=95 --cov=eventtracking
pytest --verbose -s -k 'integration'
pycodestyle --config=setup.cfg eventtracking setup.py
pylint --rcfile=pylintrc eventtracking setup.py