-
Notifications
You must be signed in to change notification settings - Fork 272
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
Remove iso8601 dependency #1176
Conversation
sig does not use iso8601: no need to silence its logging Signed-off-by: Jussi Kukkonen <[email protected]>
Our 'expires' strings are constrained by the ISO8601_DATETIME_SCHEMA which matches regex '\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z'. This can be parsed with just a datetime.strptime(): iso8601 module is not needed. * Add formats.expiry_string_to_datetime() helper function * Modify the 3 locations that used iso8601 and the api/metadata.py usage of datetime.strptime() * Remove related unnecessary logger setup * Add the missing exception documentation to relevant functions (in many cases the exception is rather unlikely as the schema has been verified many times before this though...) Fixes theupdateframework#1065 Signed-off-by: Jussi Kukkonen <[email protected]>
@@ -294,9 +294,8 @@ def from_dict(cls, signed_dict: JsonDict) -> 'Signed': | |||
# Convert 'expires' TUF metadata string to a datetime object, which is | |||
# what the constructor expects and what we store. The inverse operation | |||
# is implemented in 'to_dict'. | |||
signed_dict['expires'] = datetime.strptime( | |||
signed_dict['expires'], | |||
"%Y-%m-%dT%H:%M:%SZ").replace(tzinfo=None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note that I'm not doing the replace() in my version: I could not think of a case where it would be useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes perfect sense. I guess I thought that strptime
might adopt the system timezone. Thanks for removing unnecessary code!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Impeccable work! Thanks, @jku!
@@ -294,9 +294,8 @@ def from_dict(cls, signed_dict: JsonDict) -> 'Signed': | |||
# Convert 'expires' TUF metadata string to a datetime object, which is | |||
# what the constructor expects and what we store. The inverse operation | |||
# is implemented in 'to_dict'. | |||
signed_dict['expires'] = datetime.strptime( | |||
signed_dict['expires'], | |||
"%Y-%m-%dT%H:%M:%SZ").replace(tzinfo=None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes perfect sense. I guess I thought that strptime
might adopt the system timezone. Thanks for removing unnecessary code!
Our 'expires' strings are constrained by the ISO8601_DATETIME_SCHEMA
(meaning regex '\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}Z'). This can be
parsed with just a datetime.strptime(): iso8601 module is not needed.
of datetime.strptime()
cases the exception is rather unlikely as the schema has been verified
many times before this though...)
Fixes #1065
The reason for doing this is again vendoring into pip: I'd like to not have to vendor iso8601 as well.