Azure / azure-dev

A developer CLI that reduces the time it takes for you to get started on Azure. The Azure Developer CLI (azd) provides a set of developer-friendly commands that map to key stages in your workflow - code, build, deploy, monitor, repeat.
https://aka.ms/azd
MIT License
396 stars 189 forks source link

Standardize logic on when to log informational/warning messages to standard out #86

Open weikanglim opened 2 years ago

weikanglim commented 2 years ago

If azd is intended to be consumed by other software/tooling as part of automation, we need to produce predictable clean output as part of some well-defined contract.

For example, perhaps we intend that any software tooling that consumes azd should pass --output json for a deterministic output. If so, anytime a command has --output json format specified, the only output logged to stdout should either be:

  1. The successful result as JSON
  2. An error message describing the failure (non-JSON)

Info/warning messages that are emitted as part of the command that would usually serve to provide user feedback should be suppressed.

If we can define this logic cleanly, we would be able to increase user feedback where needed without breaking contracts for software consumers. An example would be a login attempt message that occurs with all commands, but only when allowed.

This would also be a chance to clean up existing output where we might have muddied the contract for a software consume. For example, currently parsing the result of azd version might be problematic when azd is out-of-date, and additional stdout output message is generated for the version warning:

PS D:\repos\azd-sources\test> azd version
azd version 0.0.0-dev.1 (commit 0000000000000000000000000000000000000000)
warning: your version of azd is out of date, you have 0.0.0-dev.1 and the latest version is 0.1.0-beta.1

To update to the latest version, run:
powershell -c "Set-ExecutionPolicy Bypass Process -Force; irm 'https://aka.ms/install-azd.ps1' | iex"
weikanglim commented 2 years ago

Another more obvious issue that indicates we need to do something:

Our current logic on detecting whether we should be printing interactive output is interactive := formatter.Kind() == output.NoneFormat. This is different than az cli where output == None means nothing except for errors and warnings: https://docs.microsoft.com/en-us/cli/azure/format-output-azure-cli

karolz-ms commented 2 years ago

Our NoneFormat does not mean "no output", it means "data will be outputted in default/natural format". So it is a different concept from the one that az CLI has. Feel free to propose a better name for ours.

The original Unix design is that stdout is for data, and stderr is for human interaction (not just errors!). Unfortunately, that design has not been followed as it should have been, and lots of programs dump a mix of data and messages to stdout (or assume that if something appears on stderr, that alone means an error occurred). But that does not mean we shouldn't do the right thing.

This means, when --output json is set:

  1. Data (and only data) should go to stdout
  2. Any log/warning/info messages should go to stderr
  3. Command success/failure should be indicated by the exit code (and not by something that appears either on stdout, or on stderr).
rajeshkamal5050 commented 2 years ago

@jongio @savannahostrowski Doesn't seem to be a must-fix for our next milestone. Moving it to On Deck for now.

ellismg commented 1 year ago

Wei noticed that we are somewhat inconsistent on where we write error output. When we address this issue we would try to fix that as well.