fix 1367: :khal crashes with zoneinfo attrib error adding new event #1374
Conversation
There was a problem hiding this comment.
I created an event without this patch and with this patch. The serialised timezones are not the same.
Without this patch:
BEGIN:VTIMEZONE
TZID:Europe/Amsterdam
BEGIN:STANDARD
DTSTART:20241027T020000
TZNAME:CET
TZOFFSETFROM:+0200
TZOFFSETTO:+0100
END:STANDARD
BEGIN:DAYLIGHT
DTSTART:20250330T030000
TZNAME:CEST
TZOFFSETFROM:+0100
TZOFFSETTO:+0200
END:DAYLIGHT
END:VTIMEZONEWith this patch:
BEGIN:VTIMEZONE
TZID:Europe/Amsterdam
BEGIN:STANDARD
DTSTART;TZID=Europe/Amsterdam:20241203T100000
TZNAME:1:00:00
TZOFFSETFROM:+0100
TZOFFSETTO:+0100
END:STANDARD
END:VTIMEZONEdiff:
@@ -1,9 +1,15 @@
BEGIN:VTIMEZONE
TZID:Europe/Amsterdam
BEGIN:STANDARD
-DTSTART;TZID=Europe/Amsterdam:20241203T100000
-TZNAME:1:00:00
-TZOFFSETFROM:+0100
+DTSTART:20241027T020000
+TZNAME:CET
+TZOFFSETFROM:+0200
TZOFFSETTO:+0100
END:STANDARD
+BEGIN:DAYLIGHT
+DTSTART:20250330T030000
+TZNAME:CEST
+TZOFFSETFROM:+0100
+TZOFFSETTO:+0200
+END:DAYLIGHT
END:VTIMEZONEFrom the definition for VTIMEZONE in https://www.rfc-editor.org/rfc/rfc5545#section-3.6.5:
The mandatory "DTSTART" property gives the effective onset date
and local time for the time zone sub-component definition.
"DTSTART" in this usage MUST be specified as a date with a local
time value.
If I understand correctly, the new output seems to be valid. It's accepted and rendered properly with Fastmail, no idea about other clients.
The TZNAMEs are lost, but I'm not sure how important they are of if any client actually renders these anyway.
| @property | ||
| def raw(self) -> str: | ||
| """Creates a VCALENDAR containing VTIMEZONEs | ||
| """ | ||
| """Creates a VCALENDAR containing VTIMEZONEs.""" | ||
| calendar = self._create_calendar() | ||
| tzs = [] |
There was a problem hiding this comment.
non-blocking nitpick: I would use a set here, so we can just insert new entries without having any duplicates (inserting an existing element into a set is a no-op):
| tzs = [] | |
| tzs = {} |
| tz: pytz.BaseTzInfo, | ||
| first_date: Optional[dt.datetime]=None, | ||
| last_date: Optional[dt.datetime]=None | ||
| tz, |
There was a problem hiding this comment.
Please leave type hints here:
| tz, | |
| tz: pytz.tzinfo | zoneinfo.ZoneInfo, |
| first_date = dt.datetime.today() if not first_date else first_date | ||
| last_date = first_date + dt.timedelta(days=1) if not last_date else last_date |
There was a problem hiding this comment.
I'm not entirely familiar with this code. Why did you remove to_naive_utc here?
| for subcomp in timezones.values(): | ||
| elif isinstance(tz, pytz.BaseTzInfo): | ||
| # Use pytz's transitions | ||
| transitions = tz._utc_transition_times # Avoid using internal attributes directly. |
There was a problem hiding this comment.
| transitions = tz._utc_transition_times # Avoid using internal attributes directly. | |
| transitions = tz._utc_transition_times # TODO: Avoid using internal attributes directly. |
The original code was designed to work only with pytz.BaseTzInfo objects, which caused problems when zoneinfo.ZoneInfo objects (introduced with Python 3.9) were used. The solution was to dynamically check which type of timezone object was being used (pytz or zoneinfo), and treat each case differently.
Key changes :
Check timezone type:
If the timezone is a pytz instance, the code applies the methods adapted to pytz to extract time transition information (e.g.: tz._utc_transition_times).
If the timezone is an instance of zoneinfo.ZoneInfo, the code must handle the retrieval of transition information differently, as zoneinfo does not contain the same internal attributes (such as _tzinfos and _utc_transition_times).
The create_timezone function has been refactored to accept both pytz and zoneinfo.ZoneInfo objects as arguments and to generate a VTIMEZONE component based on the transition information of both timezone types.
Key changes :
zoneinfo.ZoneInfo support: logic that depended on pytz internal attributes (such as _tzinfos and _utc_transition_times) has been replaced by zoneinfo-compatible management. For example, for zoneinfo, the _transitions() method can be used to obtain time transitions, although this is not directly supported as with pytz.
VTIMEZONE generation for both timezone types: Whether the timezone is of the pytz or zoneinfo type, a VTIMEZONE component is created, including the necessary information such as TZNAME, DTSTART, and offsets (start and end) for each time transition.
For each timezone, a VTIMEZONE must include time transitions relating to a specific period of time (often associated with the first and last date of an event). The code has been adjusted to dynamically retrieve the event start_date if available. This date is then used to correctly generate the relevant time transitions.
Key change:
Dynamic management of start_date: If self.start (the event start date) is defined, it is used to calculate the period during which time transitions should be generated. Otherwise, the current date is used as the default value.
The raw method generates the complete VCALENDAR file, including the VTIMEZONE components for each event. It has been updated to include timezone checks (pytz and zoneinfo), as well as adding VTIMEZONE components only if the timezone is not UTC.
Key change:
Timezone collection: The method scans events to collect all unique timezones (those associated with DTSTART and DTEND). It then adds the corresponding VTIMEZONE components to the calendar, by calling the create_timezone function.
Adding events (VEVENT): After adding the VTIMEZONE components, the events themselves are added to the calendar.