Add missing mandatory fields to user generated components #390
Conversation
| .. code-block:: python | ||
|
|
||
| #!/usr/bin/env python3 | ||
| from dateutil import tz |
There was a problem hiding this comment.
We are already using pytz and hopefully soon using tzinfo, let's not start using dateutil's tz implementation as well.
Also, at least in this snippet, we don't really need it, or am I missing something?
|
Hi @jacadzaca , I think the issue you tackle here is really important (making it easier, that users don't produce invalid icalendar files here). I would prefer another approach here, that -- after perhaps one or two major release versions -- assures that users cannot produce invalid icalendar files. The first versions should be opt-in, e.g. adding a validate=True parameter to icalendar.to_ical(), but later versions should either refuse to produce invalid files or add the required properties. We had i previous discussion on this before which I now can't find. Anyway, perhaps there are other opinions. |
Creating proper VTIMEZONE component is pretty hard, there is currently no code at all for it in icalendar. There is a "solution" in khal, but it's not pretty, prone to breakage and only works for pytz timezones. |
|
I agree that the goal is to stop users from creating bad icals. I think what way we choose to do it should depend on whether we want/need to refactor the parsing/marshalling code - to me the code is a little all over the place right now, thus implementing your idea might prove difficult If we'd be happy with leaving the code as is, specific functions for creating components (and then documenting that they are the way) is enough, otherwise your idea seems reasonable. If icalendar were to stop users from creating bad icals altogether, I think the proper way would be to refuse to produce output - I don't know how we could guess what timezone to put into the Timezone object or what's the correct default trigger for an alarm. |
The discussions: |
|
@geier I would assign this to you as it takes more time for me to work myself into this matter. |
|
@jacadzaca for the progress: If you like me to look at this, I will but I would like you to tell me. |
|
@niccokunzmann Since the VTIMEZONE component creation isn't correct and generating RFC complaint components isn't difficult to do by hand, I think we shouldn't merge it. Later, we can move the code into the examples section as per #443. What do you think? |
|
I added a link to this in the issue. |
This pull requests attempts to solve #315 by exposing a new ComponentFactory that's supposed to be 'user-firendly'. I added some examples in the README.
I think it wouldn't be correct to make the old ComponentFactory return components with all the required fields, since that might overwrite some properties during parsing that we wouldn't want to overwritten, e.g DTSTAMP on events. Moreover, if parsed components don't provide them, I don't think a parser should fix that implicitly, but rather throw an error.
Lastly, I need some help with figuring out what should be put in the DTSTART field inside the TimeZoneStandard/TimeZoneDaylight component, the RFC says that it ought to be 'the effective onset date and local time for the time zone sub-component', but I have no clue how to get the value using the libraries.