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.

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

Generic Update enhancements. #740

Merged
merged 2 commits into from
Aug 30, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
63 changes: 46 additions & 17 deletions src/azure/cli/commands/arm.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@
from azure.cli.commands.client_factory import get_mgmt_service_client
from azure.mgmt.resource.resources import ResourceManagementClient
from azure.cli.application import APPLICATION, IterateValue
import azure.cli._logging as _logging
from azure.cli._util import CLIError

logger = _logging.get_az_logger(__name__)

regex = re.compile('/subscriptions/(?P<subscription>[^/]*)/resourceGroups/(?P<resource_group>[^/]*)'
'/providers/(?P<namespace>[^/]*)/(?P<type>[^/]*)/(?P<name>[^/]*)'
Expand Down Expand Up @@ -285,31 +287,38 @@ def __call__(self, parser, namespace, values, option_string=None):
arg_group=group_name)
main_command_table[name] = cmd

index_regex = re.compile(r'\[(.*)\]')
index_or_filter_regex = re.compile(r'\[(.*)\]')
def set_properties(instance, expression):
key, value = expression.split('=', 1)
key, value = expression.rsplit('=', 1)

try:
value = json.loads(value)
except: #pylint:disable=bare-except
pass

name, path = _get_name_path(key)
parent_name = path[-1] if path else 'root'
root = instance
instance = _find_property(instance, path)
if instance is None:
parent = _find_property(root, path[:-1])
set_properties(parent, '{}={{}}'.format(path[-1]))
set_properties(parent, '{}={{}}'.format(parent_name))
instance = _find_property(root, path)

match = index_regex.match(name)
match = index_or_filter_regex.match(name)
index_value = int(match.group(1)) if match else None
try:
if index_value is not None:
instance[index_value] = value
elif isinstance(instance, dict):
instance[name] = value
elif isinstance(instance, list):
show_options(instance, name, key.split('.'))
elif hasattr(instance, name):
setattr(instance, name, value)
else:
logger.warning(
"Property '%s' not found on %s. Update may be ignored.", name, parent_name)
setattr(instance, name, value)
except IndexError:
raise CLIError('index {} doesn\'t exist on {}'.format(index_value, make_camel_case(name)))
Expand Down Expand Up @@ -357,12 +366,16 @@ def remove_properties(instance, argument_values):

def show_options(instance, part, path):
options = instance.__dict__ if hasattr(instance, '__dict__') else instance
options = options.keys() if isinstance(options, dict) else options
options = [make_camel_case(x) for x in options]
parent = make_camel_case('.'.join(path[:-1]).replace('.[', '['))
if isinstance(options, dict):
options = options.keys()
options = sorted([make_camel_case(x) for x in options])
elif isinstance(options, list):
options = 'index into the collection "{}" with [<index>] or [<key=value>]'.format(parent)
else:
options = sorted([make_camel_case(x) for x in options])
raise CLIError('Couldn\'t find "{}" in "{}". Available options: {}'
.format(make_camel_case(part),
make_camel_case('.'.join(path[:-1]).replace('.[', '[')),
sorted(list(options), key=str)))
.format(make_camel_case(part), parent, options))

snake_regex_1 = re.compile('(.)([A-Z][a-z]+)')
snake_regex_2 = re.compile('([a-z0-9])([A-Z])')
Expand All @@ -384,22 +397,38 @@ def _get_internal_path(path):
_path = path.split('.') \
if '.[' in path \
else path.replace('[', '.[').split('.')
return [make_snake_case(x) for x in _path]
final_paths = []
for x in _path:
final_paths.append(x if x.startswith('[') else make_snake_case(x))
return final_paths

def _get_name_path(path):
pathlist = _get_internal_path(path)
return pathlist.pop(), pathlist

def _update_instance(instance, part, path):
try:
index = index_regex.match(part)
index = index_or_filter_regex.match(part)
if index:
try:
index_value = int(index.group(1))
instance = instance[index_value]
except IndexError:
raise CLIError('index {} doesn\'t exist on {}'.format(index_value,
make_camel_case(path[-2])))
if '=' in index.group(1):
key, value = index.group(1).split('=')
matches = [x for x in instance if isinstance(x, dict) and x.get(key, None) == value]
Copy link
Contributor

@BurtBiel BurtBiel Aug 24, 2016

Choose a reason for hiding this comment

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

x.get(key, None) == value [](start = 74, length = 25)

to match an empty property, I believe this needs to be x.get(key, '') #WontFix

Copy link
Member Author

Choose a reason for hiding this comment

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

The intent here wasn't to match empty string. It would be very odd to have the key be empty string.

{
'': 'blah'
}

The purpose was to make sure that they wouldn't match if the key wasn't found.


In reply to: 76155025 [](ancestors = 76155025)

if len(matches) == 1:
instance = matches[0]
elif len(matches) > 1:
raise CLIError("non-unique key '{}' found multiple matches on {}. "
"Key must be unique.".format(
key, make_camel_case(path[-2])))
else:
raise CLIError("item with value '{}' doesn\'t exist for key '{}' on {}".format(
value, key, make_camel_case(path[-2])))
else:
try:
index_value = int(index.group(1))
instance = instance[index_value]
except IndexError:
raise CLIError('index {} doesn\'t exist on {}'.format(
index_value, make_camel_case(path[-2])))
elif isinstance(instance, dict):
instance = instance[part]
else:
Expand Down
22 changes: 18 additions & 4 deletions src/azure/cli/tests/test_generic_update.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def test_generic_update(self):
'list': [
'a',
'b',
['c', {'d': 'e'}]
['c', {'d': 'e'}, {'d': 'f'}]
]
}

Expand Down Expand Up @@ -66,6 +66,10 @@ def my_set(**kwargs): #pylint:disable=unused-argument
self.assertEqual(my_obj['list'][1][0]['key2'], 'value2',
'add a second value to the new list')

app.execute('gencommand --set list[2][d=f].d=g'.split())
self.assertEqual(my_obj['list'][2][2]['d'], 'g',
'index dictionary by unique key=value')

app.execute('gencommand --add list i=j k=l'.split())
self.assertEqual(my_obj['list'][-1]['k'], 'l',
'add multiple values to a list at once (verify last element)')
Expand Down Expand Up @@ -95,7 +99,7 @@ def test_generic_update_errors(self):
'list': [
'a',
'b',
['c', {'d': 'e'}]
['c', {'d': 'e'}, {'d': 'e'}]
]
}

Expand Down Expand Up @@ -128,7 +132,8 @@ def _execute_with_error(command, error, message):
'remove non-existent property by index')

remove_prop_message = """Couldn't find "doesntExist" in "list.doesntExist".""" + \
""" Available options: [['c', {'d': 'e'}], 'a', 'b']"""
""" Available options: index into the """ + \
"""collection "list.doesntExist" with [<index>] or [<key=value>]"""
_execute_with_error('gencommand --a1 1 --a2 2 --remove list.doesnt_exist.missing 2',
remove_prop_message,
'remove non-existent sub-property by index')
Expand All @@ -138,7 +143,8 @@ def _execute_with_error(command, error, message):
'remove out-of-range index')

set_on_list_message = """Couldn't find "doesntExist" in "list".""" + \
""" Available options: [['c', {'d': 'e'}], 'a', 'b']"""
""" Available options: index into the collection "list" with""" + \
""" [<index>] or [<key=value>]"""
_execute_with_error('gencommand --a1 1 --a2 2 --set list.doesnt_exist=foo',
set_on_list_message,
'set shouldn\'t work on a list')
Expand All @@ -150,6 +156,14 @@ def _execute_with_error(command, error, message):
"index 3 doesn't exist on [2]",
'index out of range in path')

_execute_with_error('gencommand --a1 1 --a2 2 --set list[2][d=e].doesnt_exist=foo',
"non-unique key 'd' found multiple matches on [2]. Key must be unique.",
'indexing by key must be unique')

_execute_with_error('gencommand --a1 1 --a2 2 --set list[2][f=a].doesnt_exist=foo',
"item with value 'a' doesn\'t exist for key 'f' on [2]",
'no match found when indexing by key and value')

def test_generic_update_ids(self):
my_objs = [
{
Expand Down