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

Update to support Django versions < 5.0 #16

Merged
merged 5 commits into from
Sep 19, 2023

Conversation

Afani97
Copy link
Contributor

@Afani97 Afani97 commented Jul 17, 2023

  • Update django to latest 4.2 version
  • Set a max version allowed for smpplib until a bug fix PR has been merged, noted in the toml file.
  • Update pre-commit file to include django-upgrade

pyproject.toml Outdated
@@ -14,9 +14,10 @@ include = [ { path = "tests", format = "sdist" } ]
[tool.poetry.dependencies]
python = ">=3.8"
RapidSMS = ">=2.0"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

RapidSMS might need to be upgraded, it seems to be crashing this build: https://github.com/caktus/rapidsms-smpp-gateway/actions/runs/5581453954/jobs/10199938046?pr=16#step:6:67

Copy link
Member

@tobiasmcnulty tobiasmcnulty left a comment

Choose a reason for hiding this comment

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

In addition to the rapidsms fix, it could be worth adding tox to this repo, to test against both Django 3.2 and 4.2 (and different versions of Python). See django_bread for an example.

Copy link
Member

@ronardcaktus ronardcaktus left a comment

Choose a reason for hiding this comment

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

Looks good. Maybe changes.md should be updated with release notes?

@@ -9,6 +9,11 @@ repos:
language: system
pass_filenames: false
types: ["file", "python"]
- repo: https://github.com/adamchainz/django-upgrade
rev: 1.14.0
Copy link
Member

Choose a reason for hiding this comment

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

How would you feel about updating other hooks? flake8, isort, etc to the latest version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That sounds good to me, I'll see what I can do.

@ronardcaktus
Copy link
Member

ronardcaktus commented Aug 7, 2023

Python 3.9 is failing in Github actions with the following error:

  File "/home/runner/.cache/pypoetry/virtualenvs/smpp-gateway-28Xge_hh-py3.10/lib/python3.10/site-packages/rapidsms/templatetags/paginator_tags.py", line 19, in <module>
    if "django.template.context_processors.request" not in settings.TEMPLATES[0]['OPTIONS']['context_processors']:
IndexError: list index out of range

However, settings.TEMPLATES[0]["OPTIONS"]["context_processors"] is in the code. I did some local testing:

(Pdb) settings.TEMPLATES[0]["OPTIONS"]["context_processors"]
['django.contrib.auth.context_processors.auth', 'django.template.context_processors.debug', 
'django.template.context_processors.i18n', 'django.template.context_processors.media', 'django.template.context_processors.static', 'django.template.context_processors.tz', 'django.contrib.messages.context_processors.messages', 'django.template.context_processors.request']

(Pdb) "django.template.context_processors.request" in settings.TEMPLATES[0]["OPTIONS"]["context_processors"]
True

@Afani97
Copy link
Contributor Author

Afani97 commented Aug 7, 2023

Github actions is failing with the following error:

  File "/home/runner/.cache/pypoetry/virtualenvs/smpp-gateway-28Xge_hh-py3.10/lib/python3.10/site-packages/rapidsms/templatetags/paginator_tags.py", line 19, in <module>
    if "django.template.context_processors.request" not in settings.TEMPLATES[0]['OPTIONS']['context_processors']:
IndexError: list index out of range

However, settings.TEMPLATES[0]["OPTIONS"]["context_processors"] is in the code. I did some local testing:

(Pdb) settings.TEMPLATES[0]["OPTIONS"]["context_processors"]
['django.contrib.auth.context_processors.auth', 'django.template.context_processors.debug', 
'django.template.context_processors.i18n', 'django.template.context_processors.media', 'django.template.context_processors.static', 'django.template.context_processors.tz', 'django.contrib.messages.context_processors.messages', 'django.template.context_processors.request']

(Pdb) "django.template.context_processors.request" in settings.TEMPLATES[0]["OPTIONS"]["context_processors"]
True

Yes, this PR depends on RapidSMS being updated to support Django 4.2 then this should go away. But the PR for that upgrade is failing for 3.10 for some odd reason.

@Afani97 Afani97 force-pushed the add-support-for-django-4-2 branch 2 times, most recently from 1ff3fc8 to 7f4467b Compare August 30, 2023 12:52
@tobiasmcnulty tobiasmcnulty merged commit dacc595 into main Sep 19, 2023
3 checks passed
@tobiasmcnulty tobiasmcnulty deleted the add-support-for-django-4-2 branch September 19, 2023 18:24
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.

3 participants