Skip to content

Conversation

@jseely
Copy link
Contributor

@jseely jseely commented Sep 26, 2019

Currently the Azure CLI does not work in environments without public internet access when trying to use the managed disk image in #5 of the _parse_image_argument function.

This fix ignores the ConnectionError in #4 when trying to lookup the aliases doc from githubusercontent.com and proceeds to #5. If #5 does not match then it will return the error message:

Failed to connect to remote source of image aliases. Invalid image "{}". Use a valid image URN, customer image name, customer image id, or VHD blob URI.
See vm create -h for more information on specifying an image.

This checklist is used to make sure that common guidelines for a pull request are followed.

  • The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).

  • I adhere to the Command Guidelines.

Currently the Azure CLI does not work in environments without public internet access when trying to use the managed disk image in Azure#5 of the _parse_image_argument function. 

This fix ignores the ConnectionError in Azure#4 when trying to lookup the aliases doc from `githubusercontent.com` and proceeds to Azure#5. If Azure#5 does not match then it will return the error message:
```
Failed to connect to remote source of image aliases. Invalid image "{}". Use a valid image URN, customer image name, customer image id, or VHD blob URI.
See vm create -h for more information on specifying an image.
```
@jseely jseely requested a review from qwordy September 26, 2019 16:50
@msftclas
Copy link

msftclas commented Sep 26, 2019

CLA assistant check
All CLA requirements met.

namespace.os_version = matched['version']
return 'urn'
except requests.exceptions.ConnectionError:
pass
Copy link
Contributor

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

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

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.

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

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

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.

@jseely
Copy link
Contributor Author

jseely commented Oct 7, 2019

@qwordy any update here? Is this good to merge?

@qwordy qwordy merged commit f2fbe33 into Azure:dev Oct 10, 2019
@qwordy
Copy link
Member

qwordy commented Oct 10, 2019

@qwordy any update here? Is this good to merge?

Merged.

@jseely
Copy link
Contributor Author

jseely commented Oct 11, 2019

@qwordy Thanks! I know our mutual customers will be happy to hear!

@qwordy
Copy link
Member

qwordy commented Oct 12, 2019

@qwordy Thanks! I know our mutual customers will be happy to hear!

You are welcome. I plan to add/update the alias file to/in project as a resource file, in each time we build and release. If can't access github, then use the local file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants