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

Missing dependency of msrestazure from azure-cli-core #20732

Closed
irvokopo opened this issue Dec 14, 2021 · 7 comments · Fixed by #20739 or #20740
Closed

Missing dependency of msrestazure from azure-cli-core #20732

irvokopo opened this issue Dec 14, 2021 · 7 comments · Fixed by #20739 or #20740
Assignees
Labels
customer-reported Issues that are reported by GitHub users external to the Azure organization. Packaging question The issue doesn't require a change to the product in order to be resolved. Most issues start as that

Comments

@irvokopo
Copy link

az feedback auto-generates most of the information requested below, as of CLI version 2.0.62

Describe the bug

Is there a missing dependency?

To Reproduce

In this code CLoudError is imported from msrestazure: https://github.com/Azure/azure-cli/blob/dev/src/azure-cli-core/azure/cli/core/commands/validators.py#:~:text=from%20msrestazure.azure_exceptions%20import%20CloudError

Expected behavior

Should the 'msrestazure' package be listed as a dependency here: https://github.com/Azure/azure-cli/blob/dev/src/azure-cli-core/setup.py#:~:text=%5D-,DEPENDENCIES%20%3D%20%5B,%5D,-%23%20dependencies%20for%20specific

Environment summary

Additional context

@ghost ghost added needs-triage This is a new issue that needs to be triaged to the appropriate team. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that customer-reported Issues that are reported by GitHub users external to the Azure organization. labels Dec 14, 2021
@jiasli jiasli self-assigned this Dec 15, 2021
@ghost ghost removed the needs-triage This is a new issue that needs to be triaged to the appropriate team. label Dec 15, 2021
@jiasli
Copy link
Member

jiasli commented Dec 15, 2021

@zhoxing-ms, is this line still valid after we migrate to Track 2 for azure-mgmt-resource?

from msrestazure.azure_exceptions import CloudError

@zhoxing-ms
Copy link
Contributor

Good catch! I have submitted a PR #20739 to fix this issue

@jiasli jiasli changed the title Missing dependency? Missing dependency of msrestazure from azure-cli-core Dec 15, 2021
@irvokopo
Copy link
Author

Hello,
Thank you very much for this!
Would it, maybe make sense to tackle similar usage of CloudError elsewhere as well, for e.g.:
https://github.com/Azure/azure-cli/blob/dev/src/azure-cli-core/azure/cli/core/util.py#:~:text=exceptions%20import%20JMESPathError-,from%20msrestazure.azure_exceptions%20import%20CloudError,-from%20msrest.exceptions

Thank you again!

@jiasli
Copy link
Member

jiasli commented Dec 16, 2021

Actually we can't, as some SDKs are still on Track 1. Azure CLI needs to catch those CloudError so that Track 1 SDKs' exceptions can be correctly handled.

@irvokopo
Copy link
Author

I see - thank you again for addressing the issue getting back to me - happy to have this closed : )

@irvokopo
Copy link
Author

@jiasli
Copy link
Member

jiasli commented Dec 16, 2021

Ah, nice finding. I actually just bumped packaging recently: #20661

Just wondering how you are able to find those missing dependencies? 😸

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer-reported Issues that are reported by GitHub users external to the Azure organization. Packaging question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Projects
None yet
3 participants