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: replace pytz with zoneinfo #35749

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

mumarkhan999
Copy link
Member

@mumarkhan999 mumarkhan999 commented Oct 31, 2024

Description

The alternative of some methods and variables of pytz is not available in zoneinfo.

from pytz import common_timezones, common_timezones_set, country_timezones

So currently we haven't been able to obliterate pytz. We are using them simultaneously.

@mumarkhan999 mumarkhan999 requested review from a team as code owners October 31, 2024 12:18
@mumarkhan999 mumarkhan999 removed request for a team October 31, 2024 13:57
@mumarkhan999 mumarkhan999 force-pushed the umar/remove-pytz branch 7 times, most recently from c9ecb0e to 1042fd7 Compare November 5, 2024 16:06
@mumarkhan999 mumarkhan999 force-pushed the umar/remove-pytz branch 6 times, most recently from 7f83d39 to 40229f8 Compare January 7, 2025 16:06
@mumarkhan999 mumarkhan999 force-pushed the umar/remove-pytz branch 3 times, most recently from 220144e to 6f94e9a Compare January 8, 2025 09:20
@@ -1740,8 +1740,8 @@ def _get_now(cls, browser_timezone):
# importing here to avoid a circular import
from lms.djangoapps.courseware.context_processor import user_timezone_locale_prefs
user_timezone_locale = user_timezone_locale_prefs(crum.get_current_request())
user_timezone = timezone(user_timezone_locale['user_timezone'] or browser_timezone or str(UTC))
return user_timezone.localize(datetime.now())
user_timezone = ZoneInfo(user_timezone_locale['user_timezone'] or browser_timezone or "UTC")
Copy link
Member

Choose a reason for hiding this comment

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

Is the ZoneInfo result already localized?

@@ -32,6 +32,6 @@ def test_logger_backend(caplog):
'time': '2012-05-01T07:27:01.000200+00:00',
'date': '2012-05-07'
}

print('ola-- ', saved_events[0], unpacked_event)
Copy link
Member

Choose a reason for hiding this comment

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

This debug statement can be removed now.

@@ -20,10 +19,10 @@ def default(self, obj): # lint-amnesty, pylint: disable=arguments-differ, metho
if isinstance(obj, datetime):
if obj.tzinfo is None:
# Localize to UTC naive datetime objects
obj = UTC.localize(obj) # lint-amnesty, pylint: disable=no-value-for-parameter
obj = obj.replace(tzinfo=ZoneInfo("UTC")) # lint-amnesty, pylint: disable=no-value-for-parameter
Copy link
Member

Choose a reason for hiding this comment

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

reference for this change?

return datetime.utcfromtimestamp(int(timestamp)).replace(tzinfo=utc)
return datetime.fromtimestamp(int(timestamp), tz=ZoneInfo("UTC"))
# replacing this depreciated 1099
# return datetime.utcfromtimestamp(int(timestamp)).replace(tzinfo=ZoneInfo("UTC"))
Copy link
Member

Choose a reason for hiding this comment

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

The commented line should be removed completely.

except UnknownTimeZoneError as err:
return timezone('UTC')
return ZoneInfo(user_timezone)
except KeyError:
Copy link
Member

Choose a reason for hiding this comment

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

Why not use UnknownZoneInfoError here as used in some files above?

@@ -14,6 +14,6 @@ class Migration(migrations.Migration):
migrations.AlterField(
model_name='contenttypegatingconfig',
name='enabled_as_of',
field=models.DateTimeField(blank=True, default=None, help_text='If the configuration is Enabled, then all enrollments created after this date (UTC) will be affected.', null=True, verbose_name='Enabled As Of'),
field=models.DateTimeField(blank=True, default=None, help_text='If the configuration is Enabled, then all enrollments created after this date ZoneInfo("UTC") will be affected.', null=True, verbose_name='Enabled As Of'),
Copy link
Member

Choose a reason for hiding this comment

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

No need to update migrations.

@@ -14,6 +14,6 @@ class Migration(migrations.Migration):
migrations.AlterField(
model_name='contenttypegatingconfig',
name='enabled_as_of',
field=models.DateTimeField(blank=True, default=None, help_text='If the configuration is Enabled, then all enrollments created after this date and time (UTC) will be affected.', null=True, verbose_name='Enabled As Of'),
field=models.DateTimeField(blank=True, default=None, help_text='If the configuration is Enabled, then all enrollments created after this date and time ZoneInfo("UTC") will be affected.', null=True, verbose_name='Enabled As Of'),
Copy link
Member

Choose a reason for hiding this comment

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

No need to update migrations.

@@ -14,6 +14,6 @@ class Migration(migrations.Migration):
migrations.AlterField(
model_name='coursedurationlimitconfig',
name='enabled_as_of',
field=models.DateField(blank=True, default=None, help_text='If the configuration is Enabled, then all enrollments created after this date (UTC) will be affected.', null=True, verbose_name='Enabled As Of'),
field=models.DateField(blank=True, default=None, help_text='If the configuration is Enabled, then all enrollments created after this date ZoneInfo("UTC") will be affected.', null=True, verbose_name='Enabled As Of'),
Copy link
Member

Choose a reason for hiding this comment

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

No need to update migrations.

@@ -14,6 +14,6 @@ class Migration(migrations.Migration):
migrations.AlterField(
model_name='coursedurationlimitconfig',
name='enabled_as_of',
field=models.DateTimeField(blank=True, default=None, help_text='If the configuration is Enabled, then all enrollments created after this date (UTC) will be affected.', null=True, verbose_name='Enabled As Of'),
field=models.DateTimeField(blank=True, default=None, help_text='If the configuration is Enabled, then all enrollments created after this date ZoneInfo("UTC") will be affected.', null=True, verbose_name='Enabled As Of'),
Copy link
Member

Choose a reason for hiding this comment

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

No need to update migrations.

@@ -14,6 +14,6 @@ class Migration(migrations.Migration):
migrations.AlterField(
model_name='coursedurationlimitconfig',
name='enabled_as_of',
field=models.DateTimeField(blank=True, default=None, help_text='If the configuration is Enabled, then all enrollments created after this date and time (UTC) will be affected.', null=True, verbose_name='Enabled As Of'),
field=models.DateTimeField(blank=True, default=None, help_text='If the configuration is Enabled, then all enrollments created after this date and time ZoneInfo("UTC") will be affected.', null=True, verbose_name='Enabled As Of'),
Copy link
Member

Choose a reason for hiding this comment

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

No need to update migrations.

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