> On Nov. 11, 2014, 10:55 p.m., Bill Farner wrote:
> > src/main/python/apache/aurora/client/cli/task.py, line 72
> > <https://reviews.apache.org/r/27852/diff/1/?file=757426#file757426line72>
> >
> >     Should we just make this the default behavior?  There's at least 31 
> > locations that do this, seems like a catch-all would be useful.  If we do 
> > this, i'd recommend you remove all `return EXIT_OK` lines, so only proceed 
> > if you're okay with that.
> >     
> >     Looks like the relevant changes would be in:
> >     src/main/python/apache/aurora/client/cli/standalone_client.py
> >     src/main/python/apache/aurora/client/cli/client.py
> 
> Zameer Manji wrote:
>     I don't see how this approach will work. This problem of not returning an 
> exit code comes from the subclasses not implementing the method correctly not 
> that we dispatch to the super class's implementation. In addition I'm not 
> comfortable mixing in this (minor) change with a larger structural change.
> 
> Bill Farner wrote:
>     Are we talking about different things?  I'm just suggesting a change like 
> this, around sys.exit:
>     
>     ```
>        client = AuroraCommandLine()
>        if len(sys.argv) == 1:
>          sys.argv.append("help")
>     -  sys.exit(client.execute(sys.argv[1:]))
>     +  exit_code = client.execute(sys.argv[1:])
>     +  sys.exit(0 if exit_code is None else exit_code)
>      
>      if __name__ == '__main__':
>        proxy_main()
>     ```
>     
>     This is basically just changing the contract to exit 0 if no explicit 
> exit code is returned.  Given we're working in a language that doesn't have a 
> compiler to enforce a return value, i think this is the right thing to do.
> 
> Zameer Manji wrote:
>     The python philisohpy is 'explicit over implicit'. I feel that returning 
> None is an error (that we should catch in our tests) and we should explicitly 
> returning the exit code.

Works for me, in that case, let's own up and change the code i cited above to 
assert not None.


- Bill


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/27852/#review60888
-----------------------------------------------------------


On Nov. 11, 2014, 2:50 a.m., Zameer Manji wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/27852/
> -----------------------------------------------------------
> 
> (Updated Nov. 11, 2014, 2:50 a.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-923
>     https://issues.apache.org/jira/browse/AURORA-923
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Ensure run verb returns an exit code.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/cli/task.py 
> 91175facdc8ccccc9fd59ab66781f86ee8b5940a 
>   src/test/python/apache/aurora/client/cli/BUILD 
> e1f9ebf96774b8f5c75de8570c6ba87d953ab649 
>   src/test/python/apache/aurora/client/cli/test_task_run.py 
> 8d9ef0543c1ab514d6f039ba63a1d417a4a90a1b 
> 
> Diff: https://reviews.apache.org/r/27852/diff/
> 
> 
> Testing
> -------
> 
> ./pants build src/test/python/apache/aurora/client/cli::
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Reply via email to