> On Dec. 9, 2014, 11:29 p.m., Zameer Manji wrote: > > src/main/python/apache/aurora/client/cli/__init__.py, line 384 > > <https://reviews.apache.org/r/28876/diff/1/?file=787343#file787343line384> > > > > This solution seems to be papering over the problem we have with output > > in the client. > > > > Instead of creating a new type of exception can we just prevent the > > double printing in general? > > Maxim Khutornenko wrote: > Not sure I buy it. This `print_err()` is needed when a CommandError is > raised to bail out due to some internal problem (i.e. not related to > scheduler call and not routed through context.check_and_log_response). The > fact that the same error type is used for both is exactly the reason we have > these redundant messages. There are plenty of legitimate cases where the > error needs to be logged. Consider this example: > ``` > $ aurora2 job kill devcluster/www-data/prod/hello > Error executing command: The instances list cannot be omitted in a kill > command!; use killall to kill all instances > ```
Part of the problem here is that logging to stdout/stderr happens throughout the stack. This seems to inevitably lead towards redundant output. Perhaps we should take the approach of limiting use of stdout/stderr to the top of the stack whenever possible? The only use case that seems to not fit that today seems to be client-side job updates, which we could ignore since it's scheduled for removal. - Bill ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28876/#review64460 ----------------------------------------------------------- On Dec. 9, 2014, 10:32 p.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28876/ > ----------------------------------------------------------- > > (Updated Dec. 9, 2014, 10:32 p.m.) > > > Review request for Aurora, Bill Farner and Zameer Manji. > > > Bugs: AURORA-965 > https://issues.apache.org/jira/browse/AURORA-965 > > > Repository: aurora > > > Description > ------- > > Added a new error type to raise when errors are logged explicitly. > > > Diffs > ----- > > src/main/python/apache/aurora/client/cli/__init__.py > c2eb89cae536838ac4dd46b0125a248d84f6e54c > src/main/python/apache/aurora/client/cli/context.py > ad27eb5674fb85c5d3a26a014349abdea4fa2d64 > src/test/python/apache/aurora/client/cli/test_update.py > 1f061a39279bf5ef9d4e7279016bf07e164014bb > > Diff: https://reviews.apache.org/r/28876/diff/ > > > Testing > ------- > > ./pants src/test/python/apache/aurora/client/cli:all > > Vagrant before: > > ``` > $ aurora2 job create devcluster/www-data/prod/hello > aurora/examples/jobs/hello_world.aurora > INFO] Creating job hello > Job creation failed due to error: > Job already exists: www-data/prod/hello > Error executing command: Job creation failed due to error: > ``` > > Vagrant after: > ``` > $ aurora2 job create devcluster/www-data/prod/hello > aurora/examples/jobs/hello_world.aurora > INFO] Creating job hello > Job creation failed due to error: > Job already exists: www-data/prod/hello > ``` > > > Thanks, > > Maxim Khutornenko > >
