-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Fix storage seconds #1888
Fix storage seconds #1888
Changes from 3 commits
d0c8288
803ba96
c0d876b
0a85c47
6bf7db6
c76a42a
9b16fa9
4c5fda4
ba89d48
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -575,15 +575,29 @@ def process_metric_update_namespace(namespace): | |
|
||
|
||
def datetime_string_type(string): | ||
''' Validates UTC datettime in format '%Y-%m-%d\'T\'%H:%M\'Z\''. ''' | ||
date_format = '%Y-%m-%dT%H:%MZ' | ||
return datetime.strptime(string, date_format).strftime(date_format) | ||
""" Validates UTC datettime in accepted format. Examples: 2017-12-31T01:11:59Z, | ||
2017-12-31T01:11Z or 2017-12-31T01Z or 2017-12-31 """ | ||
accepted_date_formats = ['%Y-%m-%dT%H:%M:%SZ', '%Y-%m-%dT%H:%MZ', | ||
'%Y-%m-%dT%HZ', '%Y-%m-%d'] | ||
for form in accepted_date_formats: | ||
try: | ||
return datetime.strptime(string, form).strftime(form) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems like this could be refactored to leverage the existing datetime_type method and avoid duplicating the logic here. You could merge these into one method with a parameter to toggle the behavior: get_datetime_type_method(to_string=False):
def datetime_type(string):
# stuff goes here
if to_string:
return datetime.strptime(string, form).strftime(form)
else:
return datetime.strptime(string, form)
return datetime_type |
||
except ValueError: # checks next format | ||
pass | ||
raise ValueError("Not Valid Date Format") | ||
|
||
|
||
def datetime_type(string): | ||
''' Validates UTC datettime in format '%Y-%m-%d\'T\'%H:%M\'Z\''. ''' | ||
date_format = '%Y-%m-%dT%H:%MZ' | ||
return datetime.strptime(string, date_format) | ||
""" Validates UTC datettime in accepted format. Examples: 2017-12-31T01:11:59Z, | ||
2017-12-31T01:11Z or 2017-12-31T01Z or 2017-12-31 """ | ||
accepted_date_formats = ['%Y-%m-%dT%H:%M:%SZ', '%Y-%m-%dT%H:%MZ', | ||
'%Y-%m-%dT%HZ', '%Y-%m-%d'] | ||
for form in accepted_date_formats: | ||
try: | ||
return datetime.strptime(string, form) | ||
except ValueError: # checks next format | ||
pass | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using |
||
raise ValueError("Not Valid Date Format") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should print out what didn't have a valid date format (the bad input). Otherwise the user won't know which one was screwed up. |
||
|
||
|
||
def ipv4_range_type(string): | ||
|
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.
All looks fine exception the format: "2017-12-31T01Z" looks a bit unconventional. It is not one of the format accpetable to Azure Restful API
@tjprescott thoughts?
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.
If it isn't acceptable to the API then we shouldn't allow it. However, this is in the KeyVault package. Did you mean to change this?