> 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?
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 ``` > On Dec. 9, 2014, 11:29 p.m., Zameer Manji wrote: > > src/test/python/apache/aurora/client/cli/test_update.py, line 69 > > <https://reviews.apache.org/r/28876/diff/1/?file=787345#file787345line69> > > > > Is it at all possible to avoid the use of raw Mock objects? This goes into Pystachio namespace that I am hesitant to tap into. Given the additional complexity I don't see much value in creating a concrete object for a parent-mocked container. - Maxim ----------------------------------------------------------- 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 > >
