-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Add wait commands and --no-wait support #2524
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
Changes from all commits
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 |
|---|---|---|
|
|
@@ -246,15 +246,18 @@ def arguments_loader(): | |
| def handler(args): # pylint: disable=too-many-branches,too-many-statements | ||
| from msrestazure.azure_operation import AzureOperationPoller | ||
|
|
||
| ordered_arguments = args.pop('ordered_arguments') if 'ordered_arguments' in args else [] | ||
| ordered_arguments = args.pop('ordered_arguments', []) | ||
| for item in ['properties_to_add', 'properties_to_set', 'properties_to_remove']: | ||
| if args[item]: | ||
| raise CLIError("Unexpected '{}' was not empty.".format(item)) | ||
| del args[item] | ||
|
|
||
| try: | ||
| client = factory() if factory else None | ||
| except TypeError: | ||
| client = factory(None) if factory else None | ||
|
|
||
| getterargs = {key: val for key, val in args.items() | ||
| if key in get_arguments_loader()} | ||
| getterargs = {key: val for key, val in args.items() if key in get_arguments_loader()} | ||
|
Contributor
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. What's the cost of
Member
Author
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. We are only calling it once in the handler. This method doesn't get the "argument_loader" in the sense that "get_foo" would get the foo. It returns the CLI Arguments for the "get" side of the update. There's a corresponding "set_arguments_loader" that returns the CLI Arguments for the "set" side of the update. That's also only called once in the handler. |
||
| getter = get_op_handler(getter_op) | ||
| if child_collection_prop_name: | ||
| parent = getter(client, **getterargs) if client else getter(**getterargs) | ||
|
|
@@ -278,13 +281,7 @@ def handler(args): # pylint: disable=too-many-branches,too-many-statements | |
| instance = custom_function(instance, **custom_func_args) | ||
|
|
||
| # apply generic updates after custom updates | ||
| setterargs = set_arguments_loader() | ||
| for k in args.copy().keys(): | ||
| if k in get_arguments_loader() or k in setterargs \ | ||
| or k in ('properties_to_add', 'properties_to_remove', 'properties_to_set'): | ||
| args.pop(k) | ||
| for key, val in args.items(): | ||
| ordered_arguments.append((key, val)) | ||
| setterargs = {key: val for key, val in args.items() if key in set_arguments_loader()} | ||
|
|
||
| for arg in ordered_arguments: | ||
| arg_type, arg_values = arg | ||
|
|
@@ -306,15 +303,12 @@ def handler(args): # pylint: disable=too-many-branches,too-many-statements | |
| raise CLIError('invalid syntax: {}'.format(remove_usage)) | ||
|
|
||
| # Done... update the instance! | ||
| getterargs[setter_arg_name] = parent if child_collection_prop_name else instance | ||
| setterargs[setter_arg_name] = parent if child_collection_prop_name else instance | ||
| setter = get_op_handler(setter_op) | ||
| no_wait = no_wait_param and setterargs.get(no_wait_param, None) | ||
| if no_wait: | ||
| getterargs[no_wait_param] = True | ||
|
|
||
| opres = setter(client, **getterargs) if client else setter(**getterargs) | ||
| opres = setter(client, **setterargs) if client else setter(**setterargs) | ||
|
|
||
| if no_wait: | ||
| if setterargs.get(no_wait_param, None): | ||
| return None | ||
|
|
||
| result = opres.result() if isinstance(opres, AzureOperationPoller) else opres | ||
|
|
||
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.
Shall we just say
'{}' was not expected'. I don't know much about generic update though, but in case it helpsAlso why we want to error out here? I assume we will apply their values on sending out the
updatepayload?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.
There's an Action somewhere that removes the entries in these lists and puts them into the "ordered_args" argument. So if anything was left in these, I want to raise an error so I know there's a bug in the generic update logic before I delete them.