Skip to content

Continue parsing image argument if aliases doc unreachable #10658

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

Merged
qwordy merged 3 commits into from
Oct 10, 2019
Merged

Continue parsing image argument if aliases doc unreachable #10658

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
63398b5
Continue parsing image argument if aliases doc unreachable
jseely Sep 26, 2019
e36fb5a
Updated to fix Check CLI Style
jseely Sep 26, 2019
9aa7b8a
Don't override global ConnectionError
jseely Sep 26, 2019
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
32 changes: 21 additions & 11 deletions src/azure-cli/azure/cli/command_modules/vm/_validators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,14 +254,18 @@ def _parse_image_argument(cmd, namespace):

# 4 - attempt to match an URN alias (most likely)
from azure.cli.command_modules.vm._actions import load_images_from_aliases_doc
images = load_images_from_aliases_doc(cmd.cli_ctx)
matched = next((x for x in images if x['urnAlias'].lower() == namespace.image.lower()), None)
if matched:
namespace.os_publisher = matched['publisher']
namespace.os_offer = matched['offer']
namespace.os_sku = matched['sku']
namespace.os_version = matched['version']
return 'urn'
import requests
try:
images = load_images_from_aliases_doc(cmd.cli_ctx)
matched = next((x for x in images if x['urnAlias'].lower() == namespace.image.lower()), None)
if matched:
namespace.os_publisher = matched['publisher']
namespace.os_offer = matched['offer']
namespace.os_sku = matched['sku']
namespace.os_version = matched['version']
return 'urn'
except requests.exceptions.ConnectionError:
pass
Copy link
Contributor

@yugangw-msft yugangw-msft Sep 29, 2019

Choose a reason for hiding this comment

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

we can throw the exception here with the error string you put in the 5th step, since that step won't work anyway
CC: @qwordy

Copy link
Member

@qwordy qwordy Sep 29, 2019

Choose a reason for hiding this comment

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

we can throw the exception here with the error string you put in the 5th step, since that step won't work anyway
CC: @qwordy

It can be an existing managed disk image resource.

Copy link
Contributor Author

@jseely jseely Sep 29, 2019

Choose a reason for hiding this comment

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

@yugangw-msft no, that's what we are trying to avoid here. Even on failure in #4, #5 should still be attempted.


# 5 - check if an existing managed disk image resource
compute_client = _compute_client_factory(cmd.cli_ctx)
Expand All @@ -271,9 +275,15 @@ def _parse_image_argument(cmd, namespace):
'images', 'Microsoft.Compute')
return 'image_id'
except CloudError:
err = 'Invalid image "{}". Use a valid image URN, custom image name, custom image id, VHD blob URI, or ' \
'pick an image from {}.\nSee vm create -h for more information on specifying an image.'
raise CLIError(err.format(namespace.image, [x['urnAlias'] for x in images]))
if images is not None:
err = 'Invalid image "{}". Use a valid image URN, custom image name, custom image id, ' \
'VHD blob URI, or pick an image from {}.\nSee vm create -h for more information ' \
'on specifying an image.'.format(namespace.image, [x['urnAlias'] for x in images])
else:
err = 'Failed to connect to remote source of image aliases. Invalid image "{}". Use a ' \
'valid image URN, custom image name, custom image id, or VHD blob URI.\nSee vm ' \
'create -h for more information on specifying an image.'.format(namespace.image)
Copy link
Member

@qwordy qwordy Sep 29, 2019

Choose a reason for hiding this comment

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

If it's not a valid alias nor an existing image name, what will happen?

Copy link
Contributor Author

@jseely jseely Sep 29, 2019

Choose a reason for hiding this comment

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

If it's not a valid alias then it goes to #5 as before, if it's not the name of an existing managed image it raises the same error message as before.

raise CLIError(err)


def _get_image_plan_info_if_exists(cmd, namespace):
Expand Down