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.

Sign up for GitHub

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

Jump to bottom

Django5 #179

Merged
merged 3 commits into from
Dec 6, 2023
Merged

Django5 #179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e786528
Remove pytz use, start testing on django 5.0
norkans7 Dec 5, 2023
f7289e5
Use datetime timezone aliased as tzone
norkans7 Dec 5, 2023
fff6b83
Replace self.assertEquals with self.assertEqual
norkans7 Dec 5, 2023
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
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ jobs:
matrix:
python-version: ["3.10.x", "3.11.x"]
pg-version: ["14", "15"]
django-version: ["4.1.x", "4.2.x"]
django-version: ["4.1.x", "4.2.x", "5.0.x"]
runs-on: ubuntu-latest
steps:
- name: Checkout code
Expand Down
137 changes: 63 additions & 74 deletions poetry.lock
View file
Open in desktop

Large diffs are not rendered by default.

3 changes: 1 addition & 2 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,8 @@ repository = "http://github.com/nyaruka/smartmin"

[tool.poetry.dependencies]
python = "^3.10"
Django = ">= 4.0, < 5.0"
Django = ">= 4.0, < 5.1"
celery = ">= 5.1"
pytz = "*"
redis = ">= 3.5.3"
sqlparse = "^0.4.1"
xlrd = "^1.2.0"
Expand Down
14 changes: 7 additions & 7 deletions smartmin/csv_imports/tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,16 @@ def test_csv_import(self):
pass

def test_generate_file_path(self):
self.assertEquals(generate_file_path(ImportTask(), "allo.csv"), "csv_imports/allo.csv")
self.assertEquals(generate_file_path(ImportTask(), "allo.xlsx"), "csv_imports/allo.xlsx")
self.assertEquals(generate_file_path(ImportTask(), "allo.foo.bar"), "csv_imports/allo.foo.bar")
self.assertEqual(generate_file_path(ImportTask(), "allo.csv"), "csv_imports/allo.csv")
self.assertEqual(generate_file_path(ImportTask(), "allo.xlsx"), "csv_imports/allo.xlsx")
self.assertEqual(generate_file_path(ImportTask(), "allo.foo.bar"), "csv_imports/allo.foo.bar")

long_name = "foo" * 100

test_file_name = "%s.xls.csv" % long_name
self.assertEquals(len(generate_file_path(ImportTask(), test_file_name)), 100)
self.assertEquals(generate_file_path(ImportTask(), test_file_name), "csv_imports/%s.csv" % long_name[:84])
self.assertEqual(len(generate_file_path(ImportTask(), test_file_name)), 100)
self.assertEqual(generate_file_path(ImportTask(), test_file_name), "csv_imports/%s.csv" % long_name[:84])

test_file_name = "%s.abc.xlsx" % long_name
self.assertEquals(len(generate_file_path(ImportTask(), test_file_name)), 100)
self.assertEquals(generate_file_path(ImportTask(), test_file_name), "csv_imports/%s.xlsx" % long_name[:83])
self.assertEqual(len(generate_file_path(ImportTask(), test_file_name)), 100)
self.assertEqual(generate_file_path(ImportTask(), test_file_name), "csv_imports/%s.xlsx" % long_name[:83])
6 changes: 2 additions & 4 deletions smartmin/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
import json
import traceback
import zoneinfo
from datetime import datetime
from datetime import datetime, timezone as tzone

from xlrd import XL_CELL_DATE, XLRDError, open_workbook, xldate_as_tuple

Expand Down Expand Up @@ -220,9 +220,7 @@ def import_xls(cls, filename, user, import_params, log=None, import_results=None
# timezone for date cells can be specified as an import parameter or defaults to UTC
# use now to determine a relevant timezone
naive_timezone = (
zoneinfo.ZoneInfo(import_params["timezone"])
if import_params and "timezone" in import_params
else timezone.utc
Copy link
Member

Choose a reason for hiding this comment

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

I'm confused.. isn't timezone.utc fine to use? I think we just need to set USE_DEPRECATED_PYTZ = False

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 was an alias to datetime.timezone.utc and that is removed in django 5.0

Copy link
Contributor Author

@norkans7 norkans7 Dec 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

USE_DEPRECATED_PYTZ is removed as well

Copy link
Member

Choose a reason for hiding this comment

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

Arrghh all this time I've been replacing pytz.UTC with django.utils.timezone.utc

I see the old warning message now is:

The django.utils.timezone.utc alias is deprecated. 
Please update your code to use datetime.timezone.utc instead.

So maybe we should be using datetime.timezone.utc ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had tried that in a54199d but I did not like that since I had to alias datetime.timezone to not collide with django.utils.timezone still used for things like timezone.now()

Copy link
Member

Choose a reason for hiding this comment

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

I think I'm leaning toward always accessing it like datetime.timezone.utc and then continuing to use timezone.now() like that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well I guess on our side we can just decide to consistently use the alias approach for now
from datetime import timezone as datetime_timezone

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I agree we need to keep timezone.now() but datetime.timezone.utc has issues since we mostly import
from datetime import datetime, timedelta

I think we can use the alias for timezone
from datetime import timezone as datetime_timezone

That will easily for for the existing imports

Copy link
Member

Choose a reason for hiding this comment

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

Can we pick something shorter than datetime_timezone? tzone?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I'll use tzone

zoneinfo.ZoneInfo(import_params["timezone"]) if import_params and "timezone" in import_params else tzone.utc
)
tz = timezone.now().astimezone(naive_timezone).tzinfo

Expand Down
8 changes: 4 additions & 4 deletions smartmin/templatetags/smartmin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import json
import re
import zoneinfo
from datetime import datetime, timedelta
from datetime import datetime, timedelta, timezone as tzone

from django import template
from django.conf import settings
Expand Down Expand Up @@ -34,7 +34,7 @@ def format_datetime(time):
"""
user_time_zone = timezone.get_current_timezone()
if time.tzinfo is None:
time = time.replace(tzinfo=timezone.utc)
time = time.replace(tzinfo=tzone.utc)
user_time_zone = zoneinfo.ZoneInfo(getattr(settings, "USER_TIME_ZONE", "GMT"))

time = time.astimezone(user_time_zone)
Expand Down Expand Up @@ -164,7 +164,7 @@ def map(string, args):
@register.filter
def gmail_time(dtime, now=None):
if dtime.tzinfo is None:
dtime = dtime.replace(tzinfo=timezone.utc)
dtime = dtime.replace(tzinfo=tzone.utc)
user_time_zone = zoneinfo.ZoneInfo(getattr(settings, "USER_TIME_ZONE", "GMT"))
dtime = dtime.astimezone(user_time_zone)
else:
Expand All @@ -174,7 +174,7 @@ def gmail_time(dtime, now=None):
now = timezone.now()

if now.tzinfo is None:
now = now.replace(tzinfo=timezone.utc)
now = now.replace(tzinfo=tzone.utc)

twelve_hours_ago = now - timedelta(hours=12)

Expand Down
8 changes: 4 additions & 4 deletions smartmin/tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@ def fetch_protected(self, url, user, post_data=None, failOnFormValidation=True):
# but now we can!
if not post_data:
response = self.client.get(url)
self.assertEquals(200, response.status_code)
self.assertEqual(200, response.status_code)
else:
response = self.client.post(url, data=post_data)
self.assertNotRedirect(response, reverse("users.user_login"), msg="Unexpected redirect to login")

if failOnFormValidation:
self.assertNoFormErrors(response, post_data)
self.assertEquals(302, response.status_code)
self.assertEqual(302, response.status_code)

return response

Expand Down Expand Up @@ -162,7 +162,7 @@ def testDelete(self):
return
object = self.getTestObject()
self._do_test_view("delete", object, post_data=dict())
self.assertEquals(0, len(self.getManager().filter(pk=object.pk)))
self.assertEqual(0, len(self.getManager().filter(pk=object.pk)))

def testList(self):
if "list" not in self.getCRUDL().actions:
Expand Down Expand Up @@ -222,7 +222,7 @@ def _do_test_view(self, action=None, object=None, post_data=None, query_string=N
def assertPageGet(self, action, response):
if response.status_code == 302:
self.fail("'%s' resulted in an unexpected redirect to: %s" % (action, response.get("Location")))
self.assertEquals(
self.assertEqual(
200,
response.status_code,
)
Expand Down
Loading