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.

Sign up for GitHub

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

Jump to bottom

Fix storage seconds #1888

Merged
merged 9 commits into from
Jan 27, 2017
Merged

Fix storage seconds #1888

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
d0c8288
Fix storage bugs #1560 #1563
Jan 26, 2017
803ba96
2 space inline comment
Jan 26, 2017
c0d876b
removed else
Jan 26, 2017
0a85c47
abstracted date_string
Jan 26, 2017
6bf7db6
abstraction through nested functions
Jan 26, 2017
c76a42a
removed f
Jan 26, 2017
9b16fa9
cleared up parameters and error message
Jan 27, 2017
4c5fda4
change function calls
Jan 27, 2017
ba89d48
change function calls
Jan 27, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ src/build
/.vs/config/applicationhost.config
.vscode/settings.json
.vscode/.ropeproject/
.project
.pydevproject

# Azure deployment credentials
*.pubxml
Expand Down
8 changes: 4 additions & 4 deletions src/command_modules/azure-cli-keyvault/azure/cli/command_modules/keyvault/_params.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ def register_attributes_argument(scope, name, attr_class, create=False):
register_extra_cli_argument(scope, 'disabled', action='store_true', help='Create {} in disabled state.'.format(name))
else:
register_extra_cli_argument(scope, 'enabled', default=None, choices=['true', 'false'], help='Enable the {}.'.format(name))
register_extra_cli_argument(scope, 'expires', default=None, help='Expiration UTC datetime (Y-m-d\'T\'H:M\'Z\').', type=datetime_type)
register_extra_cli_argument(scope, 'not_before', default=None, help='Key not usable before the provided UTC datetime (Y-m-d\'T\'H:M\'Z\').', type=datetime_type)
register_extra_cli_argument(scope, 'expires', default=None, help='Expiration UTC datetime (Y-m-d\'T\'H:M:S\'Z\').', type=datetime_type)
register_extra_cli_argument(scope, 'not_before', default=None, help='Key not usable before the provided UTC datetime (Y-m-d\'T\'H:M:S\'Z\').', type=datetime_type)

# ARGUMENT DEFINITIONS

Expand Down Expand Up @@ -122,8 +122,8 @@ def register_attributes_argument(scope, name, attr_class, create=False):
register_cli_argument('keyvault key {}'.format(item), 'destination', options_list=('--protection', '-p'), choices=['software', 'hsm'], help='Specifies the type of key protection.', validator=validate_key_type, type=str.lower)
register_cli_argument('keyvault key {}'.format(item), 'disabled', action='store_true', help='Create key in disabled state.')
register_cli_argument('keyvault key {}'.format(item), 'key_size', options_list=('--size',), type=int)
register_cli_argument('keyvault key {}'.format(item), 'expires', default=None, help='Expiration UTC datetime (Y-m-d\'T\'H:M\'Z\').', type=datetime_type)
register_cli_argument('keyvault key {}'.format(item), 'not_before', default=None, help='Key not usable before the provided UTC datetime (Y-m-d\'T\'H:M\'Z\').', type=datetime_type)
register_cli_argument('keyvault key {}'.format(item), 'expires', default=None, help='Expiration UTC datetime (Y-m-d\'T\'H:M:S\'Z\').', type=datetime_type)
register_cli_argument('keyvault key {}'.format(item), 'not_before', default=None, help='Key not usable before the provided UTC datetime (Y-m-d\'T\'H:M:S\'Z\').', type=datetime_type)

register_cli_argument('keyvault key import', 'pem_file', type=file_type, help='PEM file containing the key to be imported.', arg_group='Key Source', completer=FilesCompleter(), validator=validate_key_import_source)
register_cli_argument('keyvault key import', 'pem_password', help='Password of PEM file.', arg_group='Key Source')
Expand Down
13 changes: 10 additions & 3 deletions src/command_modules/azure-cli-keyvault/azure/cli/command_modules/keyvault/_validators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,16 @@ def base64_encoded_certificate_type(string):
return cert_data

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 """
Copy link
Contributor

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?

Copy link
Member

@tjprescott tjprescott Jan 26, 2017 •
edited
Loading

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?

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
raise ValueError("Not Valid Date Format")

def vault_base_url_type(name):
from azure.cli.core._profile import CLOUD
Expand Down
4 changes: 2 additions & 2 deletions src/command_modules/azure-cli-storage/azure/cli/command_modules/storage/_params.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,8 +450,8 @@ def register_source_uri_arguments(scope):
register_path_argument('storage file url')

for item in ['container', 'share', 'table', 'queue']:
register_cli_argument('storage {} policy'.format(item), 'start', type=datetime_string_type, help='start UTC datetime (Y-m-d\'T\'H:M\'Z\'). Defaults to time of request.')
register_cli_argument('storage {} policy'.format(item), 'expiry', type=datetime_string_type, help='expiration UTC datetime in (Y-m-d\'T\'H:M\'Z\')')
register_cli_argument('storage {} policy'.format(item), 'start', type=datetime_string_type, help='start UTC datetime (Y-m-d\'T\'H:M:S\'Z\'). Defaults to time of request.')
register_cli_argument('storage {} policy'.format(item), 'expiry', type=datetime_string_type, help='expiration UTC datetime in (Y-m-d\'T\'H:M:S\'Z\')')

register_cli_argument('storage table', 'table_name', table_name_type, options_list=('--name', '-n'))

Expand Down
26 changes: 20 additions & 6 deletions src/command_modules/azure-cli-storage/azure/cli/command_modules/storage/_validators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The 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
Copy link
Member

Choose a reason for hiding this comment

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

Using continue here instead of pass would make the logic more clear and remove the need for the comment. Ultimately they do the same thing in this case.

raise ValueError("Not Valid Date Format")
Copy link
Member

Choose a reason for hiding this comment

The 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):
Expand Down
Loading