-
Notifications
You must be signed in to change notification settings - Fork 3.3k
{Feedback} Show msal and azure-mgmt-resource versions
#20754
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
Conversation
| ext_line = -1 | ||
| legal_line = -1 | ||
| for i, line in enumerate(lines): | ||
| if line.startswith("azure-cli"): | ||
| line = " ".join(line.split()) | ||
| new_lines.append(line) | ||
| if line.lower().startswith("extensions:"): | ||
| ext_line = i | ||
| continue | ||
| l_lower = line.lower() | ||
| if all(["legal" in l_lower, "docs" in l_lower, "info" in l_lower]): | ||
| legal_line = i | ||
| if 'python location' in line.lower(): | ||
| break | ||
| new_lines.append(line) | ||
|
|
||
| new_lines.append("") | ||
|
|
||
| if 0 < ext_line < legal_line: | ||
| for i in range(ext_line, legal_line): | ||
| l_lower = lines[i].lower() | ||
| if "python location" in l_lower or "extensions directory" in l_lower: | ||
| break | ||
|
|
||
| line = " ".join(lines[i].split()) | ||
| new_lines.append(line) | ||
|
|
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 is no need to parse the output of az --version in such detail. Simply remove consecutive spaces and it will work, given we don't need to minify the body of az feedback anymore (#17353).
| # Add azure-mgmt-resource version | ||
| try: | ||
| # pylint: disable=protected-access | ||
| from azure.mgmt.resource._version import VERSION as azure_mgmt_resource_version | ||
| except ImportError: | ||
| try: | ||
| from azure.mgmt.resource.version import VERSION as azure_mgmt_resource_version | ||
| except ImportError: | ||
| azure_mgmt_resource_version = "N/A" | ||
| versions['azure-mgmt-resource'] = azure_mgmt_resource_version |
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.
We also show azure-mgmt-resource version because some packages will install Track 1 SDKs, causing conflict with Azure CLI.
| versions = {} | ||
| # Add msal version | ||
| try: | ||
| from msal import __version__ as msal_version |
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.
It would work!
By the way, do you consider also printing MSAL EX? :-)
I'm still providing an approval for this part. Other Azure CLI teammates please help review the rest of this PR.
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.
By the way, do you consider also printing MSAL EX? :-)
Let's only add msal for now, since the list can go on and on.
|
Packaging |
|
Another example demonstrating we should show |
msal and azure-mgmt-resource versionsmsal and azure-mgmt-resource versions
Description
Show
msalandazure-mgmt-resourceversions, as proposed at #20723 (comment).In this way, we can easily troubleshoot issues caused by dependency issues like #20723, #20503, #20304, #20138, #20101, #19557, #19502, #19357, #19209, #19169, #19168, #18918, #18791, #18532.
(Query: https://github.com/Azure/azure-cli/issues?q=is%3Aissue+got+an+unexpected+keyword+user_agent)
After this PR:
az feedbackwill also include such information:Indeed, there may be issues related to other Track 2 SDKs or dependencies, but they happen less often and we can solve them on a per-issue basis.