aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Maxim Khutornenko" <ma...@apache.org>
Subject Re: Review Request 28876: Suppressing redundant client command error messaging.
Date Tue, 09 Dec 2014 23:47:31 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message