Instruct Azure CLI to not color output

[NVolcz]

The Azure CLI supports colored output by using colorama which resets the color after execution by printing "[0m" if the terminal supports color. This can in some cases cause the AzureCliCredential to fail for example when developing in PyCharm: https://github.com/Azure/azure-cli/issues/9903.
Azure CLI allows color output to be disabled by setting the environment variable:AZURE_CORE_NO_COLOR.

The PR in azure-cli: Azure/azure-cli#12601

[chlowell]

Hi @NVolcz, thanks for opening this! I was able to reproduce the problem locally with an older version of the CLI but not with the latest version (2.5.1). Do you still see this problem with the latest CLI?

[NVolcz]

Upgrading to Azure CLI to 2.5.1 fixed this, thanks! I was running version 2.3.1 of the CLI.

[chlowell]

Great, then it seems that while adding AZURE_CORE_NO_COLOR, someone made the default behavior of get-access-token what we want. I'll close this then. Thanks again for opening it!

[NVolcz]

It seems that this behavior has once again changed. I am now running azure-cli 2.7.0 and are getting the color reset sequence in the end of the JSON.

[chlowell]

That's interesting, I don't see it from 2.7.0 either. @jiasli, what default coloring behavior should we expect from get-access-token?

[jiasli]

I am able to repro. The testing script is very simple:

from azure.identity import AzureCliCredential

cred = AzureCliCredential()
token = cred.get_token('https://management.azure.com/')
print(token)

Add a breakpoint at:

azure-sdk-for-python/sdk/identity/azure-identity/azure/identity/_credentials/azure_cli.py

Line 57 in a7ac22a

token = parse_token(output)

In a shell, the issue doesn't occur:

> python D:\cli\testproj\demo.py
AccessToken(token='....', expires_on=1592293455)

This issue should always be there. We didn't change anything when color does get enabled.

When AzureCliCredential doesn't explicitly tell Azure CLI to disable color, Azure CLI will initiate colorama. However, when colorama detects PYCHARM_HOSTED env var is set, it doesn't query on stream.isatty anymore.

https://github.com/tartley/colorama/blob/bfa8c22e8f9b51ad3428c165087540cea5d44a1f/colorama/ansitowin32.py#L45-L47

        if 'PYCHARM_HOSTED' in os.environ:
            if stream is not None and (stream is sys.__stdout__ or stream is sys.__stderr__):
                return True

So, colorama will print the control sequence \033[0m when Azure CLI exits.

https://github.com/tartley/colorama/blob/bfa8c22e8f9b51ad3428c165087540cea5d44a1f/colorama/ansitowin32.py#L170-L174

    def reset_all(self):
        if self.convert:
            self.call_win32('m', (0,))
        elif not self.strip and not self.stream.closed:
            self.wrapped.write(Style.RESET_ALL)

I think this is more of an issue with colorama and PyCharm. For Azure CLI and Azure Identity, the best approach is to explicitly disable color in Azure CLI so that colorama never gets initiated.

I have created issue tartley/colorama#263 in colorama's repo.

[NVolcz]

Would it be crazy to suggest to reconsider my pull request in light of this impediment with colorama?

[chlowell]

Not at all, that would be entirely reasonable because it seems we must do something here 😄

If you want to fix up this pull request, my feedback on its current state is:

• the fix should also apply to non-Windows platforms and the async AzureCliCredential
• the sync credential must support Python 2.7 (e.g. no f-strings)
• I prefer using the env keyword argument to check_output and create_subprocess_exec

If not, no worries, we'll get this fixed in the next release.

[NVolcz]

I have updated my PR but haven't verified that it works on Linux yet.

[chlowell]

Thanks for the update. There are still a couple things left to get this over the finish line: I'd rather set the environment variable with the env keyword argument when starting the subprocess, and the async AzureCliCredential needs the same fix.

This needs to merge early next week to make the next release. Is that too soon for you? I can push a couple commits to finish this if you like.

[NVolcz]

How about now?

[chlowell]

Looks good to me, I'll take it 😄

Thanks for reporting this, and for fixing it!