feat(api): improve analysis cli #15027
Conversation
nope! if you want to do that, you can.
the changes in this pr are all interface-level changes that we don't really test for most of our code (including this one - there is no explicit testing for argument handling behavior. i'll make sure that the human-output stuff is parsable, but I don't intend to add exhaustive argument handling tests. |
|
okay I did add some though |
- adds a human-json-output flag that will output formatted json that is readable - adds a log-output and log-level flag to pipe logs around (default is stderr) - adds a check flag to control return code values - allows specifying stdout streams with - for file paths Closes EXEC-425
sfoster1
force-pushed
the
exec-425-improve-analysis-cli
branch
from
c4dc730
to
4295882
Compare
api/src/opentrons/cli/analyze.py
Outdated
| type=click.Path(path_type=AsyncPath), | ||
| type=click.File(mode="wb"), |
There was a problem hiding this comment.
FYI, using stdout for machine-readable output is unreliable because if we're analyzing a Python protocol, it can also print to stdout, and that will corrupt the JSON stream.
Should we print() a warning if - is specified, so it will fail early and obviously if somebody tries to rely on this?
There was a problem hiding this comment.
IMO no, but we should put a warning in the help text. Somebody will only know to use that flag by actually reading the help text, so that should be good enough.
That's a good secondary improvement too - there's no reason we can't capture stdout and save it somewhere separate.
- adds a human-json-output flag that will output formatted json that is readable - adds a log-output and log-level flag to pipe logs around (default is stderr) - adds a check flag to control return code values - allows specifying stdout streams with - for file paths Closes EXEC-425
Closes EXEC-425