You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Mark Chu-Carroll <mc...@twopensource.com> on 2014/02/04 22:49:17 UTC
Re: Review Request 17724: Add an exception catch to provide a better error
message when distributed command runner fails
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17724/
-----------------------------------------------------------
(Updated Feb. 4, 2014, 4:49 p.m.)
Review request for Aurora, Kevin Sweeney and Brian Wickman.
Summary (updated)
-----------------
Add an exception catch to provide a better error message when distributed command runner fails
Bugs: aurora-128
https://issues.apache.org/jira/browse/aurora-128
Repository: aurora
Description
-------
Add an exception catch to provide a better error message when distributed command runner fails due to un-specified executor parameters in the slave configurations.
Diffs
-----
src/main/python/apache/aurora/client/api/command_runner.py c645e9556b50b332cc5a298ceee3d9188a68a727
src/test/python/apache/aurora/client/commands/test_run.py abcc275d57373038227d3805c16b17b967f92048
Diff: https://reviews.apache.org/r/17724/diff/
Testing
-------
Manual testing on the command line.
Thanks,
Mark Chu-Carroll
Re: Review Request 17724: Add an exception catch to provide a better error
message when distributed command runner fails
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17724/#review33647
-----------------------------------------------------------
src/main/python/apache/aurora/client/api/command_runner.py
<https://reviews.apache.org/r/17724/#comment63198>
It's unclear why this should be an API-level check instead of a higher-level CLI check verifying that cluster selected expresses the trait required to use the DCR. Also, this error message isn't informative (which parameters do I need to define? where do I define them?)
- Kevin Sweeney
On Feb. 4, 2014, 1:49 p.m., Mark Chu-Carroll wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17724/
> -----------------------------------------------------------
>
> (Updated Feb. 4, 2014, 1:49 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
>
>
> Bugs: aurora-128
> https://issues.apache.org/jira/browse/aurora-128
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Add an exception catch to provide a better error message when distributed command runner fails due to un-specified executor parameters in the slave configurations.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/command_runner.py c645e9556b50b332cc5a298ceee3d9188a68a727
> src/test/python/apache/aurora/client/commands/test_run.py abcc275d57373038227d3805c16b17b967f92048
>
> Diff: https://reviews.apache.org/r/17724/diff/
>
>
> Testing
> -------
>
> Manual testing on the command line.
>
>
> Thanks,
>
> Mark Chu-Carroll
>
>
Re: Review Request 17724: Add an exception catch to provide a better error
message when distributed command runner fails
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17724/#review33849
-----------------------------------------------------------
Ship it!
src/main/python/apache/aurora/client/api/command_runner.py
<https://reviews.apache.org/r/17724/#comment63463>
cls.EnvironmentError
src/test/python/apache/aurora/client/commands/test_run.py
<https://reviews.apache.org/r/17724/#comment63464>
revert
- Kevin Sweeney
On Feb. 6, 2014, 1:40 p.m., Mark Chu-Carroll wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17724/
> -----------------------------------------------------------
>
> (Updated Feb. 6, 2014, 1:40 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
>
>
> Bugs: aurora-128
> https://issues.apache.org/jira/browse/aurora-128
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Add an exception catch to provide a better error message when distributed command runner fails due to un-specified executor parameters in the slave configurations.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/command_runner.py f7605f40b5c4ec864a1bae68649f282268260528
> src/main/python/apache/aurora/client/commands/run.py 332bded632ee4cfc0a986540239cb4cc296807ea
> src/main/python/apache/aurora/client/commands/ssh.py d1499ec1e9e910bc32f5f9f4b050c53316348503
> src/test/python/apache/aurora/client/commands/test_run.py abcc275d57373038227d3805c16b17b967f92048
>
> Diff: https://reviews.apache.org/r/17724/diff/
>
>
> Testing
> -------
>
> Manual testing on the command line.
>
>
> Thanks,
>
> Mark Chu-Carroll
>
>
Re: Review Request 17724: Add an exception catch to provide a better error
message when distributed command runner fails
Posted by Mark Chu-Carroll <mc...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17724/
-----------------------------------------------------------
(Updated Feb. 6, 2014, 4:40 p.m.)
Review request for Aurora, Kevin Sweeney and Brian Wickman.
Changes
-------
Better?
Bugs: aurora-128
https://issues.apache.org/jira/browse/aurora-128
Repository: aurora
Description
-------
Add an exception catch to provide a better error message when distributed command runner fails due to un-specified executor parameters in the slave configurations.
Diffs (updated)
-----
src/main/python/apache/aurora/client/api/command_runner.py f7605f40b5c4ec864a1bae68649f282268260528
src/main/python/apache/aurora/client/commands/run.py 332bded632ee4cfc0a986540239cb4cc296807ea
src/main/python/apache/aurora/client/commands/ssh.py d1499ec1e9e910bc32f5f9f4b050c53316348503
src/test/python/apache/aurora/client/commands/test_run.py abcc275d57373038227d3805c16b17b967f92048
Diff: https://reviews.apache.org/r/17724/diff/
Testing
-------
Manual testing on the command line.
Thanks,
Mark Chu-Carroll
Re: Review Request 17724: Add an exception catch to provide a better error
message when distributed command runner fails
Posted by Jonathan Boulle <jo...@twopensource.com>.
> On Feb. 5, 2014, 2:18 a.m., Mark Chu-Carroll wrote:
> > src/main/python/apache/aurora/client/api/command_runner.py, line 139
> > <https://reviews.apache.org/r/17724/diff/1/?file=469319#file469319line139>
> >
> > It's API level, because this is the highest level shared by all of the distinct code points that could call it. I didn't want to repeat the same try/catch multiple times in multiple places - in particular, because at some point, I'd really like to clean up the Pystachio error handling throughout the client, and having multiple copies of the same thing would make that more painful.
> >
> > The reason that it doesn't generate anything more specific is because that's already happening. In the current code, you get an error message saying "TypeError: CommandRunnerTrait[slave_run_directory] is required.". The problem that I'm trying to fix is that for users, that's not particularly clear. Now, they'll see "Error executing command: slave doesn't define necessary parameters, CommandRunnerTrain[slave_run_directory] is required".
> >
>
> Mark Chu-Carroll wrote:
> ping?
I tend to agree with both of you that it's rather suboptimal - could you add a TODO for error handling cleanup?
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17724/#review33674
-----------------------------------------------------------
On Feb. 4, 2014, 9:49 p.m., Mark Chu-Carroll wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17724/
> -----------------------------------------------------------
>
> (Updated Feb. 4, 2014, 9:49 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
>
>
> Bugs: aurora-128
> https://issues.apache.org/jira/browse/aurora-128
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Add an exception catch to provide a better error message when distributed command runner fails due to un-specified executor parameters in the slave configurations.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/command_runner.py c645e9556b50b332cc5a298ceee3d9188a68a727
> src/test/python/apache/aurora/client/commands/test_run.py abcc275d57373038227d3805c16b17b967f92048
>
> Diff: https://reviews.apache.org/r/17724/diff/
>
>
> Testing
> -------
>
> Manual testing on the command line.
>
>
> Thanks,
>
> Mark Chu-Carroll
>
>
Re: Review Request 17724: Add an exception catch to provide a better error
message when distributed command runner fails
Posted by Mark Chu-Carroll <mc...@twopensource.com>.
> On Feb. 4, 2014, 9:18 p.m., Mark Chu-Carroll wrote:
> > src/main/python/apache/aurora/client/api/command_runner.py, line 139
> > <https://reviews.apache.org/r/17724/diff/1/?file=469319#file469319line139>
> >
> > It's API level, because this is the highest level shared by all of the distinct code points that could call it. I didn't want to repeat the same try/catch multiple times in multiple places - in particular, because at some point, I'd really like to clean up the Pystachio error handling throughout the client, and having multiple copies of the same thing would make that more painful.
> >
> > The reason that it doesn't generate anything more specific is because that's already happening. In the current code, you get an error message saying "TypeError: CommandRunnerTrait[slave_run_directory] is required.". The problem that I'm trying to fix is that for users, that's not particularly clear. Now, they'll see "Error executing command: slave doesn't define necessary parameters, CommandRunnerTrain[slave_run_directory] is required".
> >
ping?
- Mark
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17724/#review33674
-----------------------------------------------------------
On Feb. 4, 2014, 4:49 p.m., Mark Chu-Carroll wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17724/
> -----------------------------------------------------------
>
> (Updated Feb. 4, 2014, 4:49 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
>
>
> Bugs: aurora-128
> https://issues.apache.org/jira/browse/aurora-128
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Add an exception catch to provide a better error message when distributed command runner fails due to un-specified executor parameters in the slave configurations.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/command_runner.py c645e9556b50b332cc5a298ceee3d9188a68a727
> src/test/python/apache/aurora/client/commands/test_run.py abcc275d57373038227d3805c16b17b967f92048
>
> Diff: https://reviews.apache.org/r/17724/diff/
>
>
> Testing
> -------
>
> Manual testing on the command line.
>
>
> Thanks,
>
> Mark Chu-Carroll
>
>
Re: Review Request 17724: Add an exception catch to provide a better error
message when distributed command runner fails
Posted by Mark Chu-Carroll <mc...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17724/#review33674
-----------------------------------------------------------
src/main/python/apache/aurora/client/api/command_runner.py
<https://reviews.apache.org/r/17724/#comment63254>
It's API level, because this is the highest level shared by all of the distinct code points that could call it. I didn't want to repeat the same try/catch multiple times in multiple places - in particular, because at some point, I'd really like to clean up the Pystachio error handling throughout the client, and having multiple copies of the same thing would make that more painful.
The reason that it doesn't generate anything more specific is because that's already happening. In the current code, you get an error message saying "TypeError: CommandRunnerTrait[slave_run_directory] is required.". The problem that I'm trying to fix is that for users, that's not particularly clear. Now, they'll see "Error executing command: slave doesn't define necessary parameters, CommandRunnerTrain[slave_run_directory] is required".
- Mark Chu-Carroll
On Feb. 4, 2014, 4:49 p.m., Mark Chu-Carroll wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17724/
> -----------------------------------------------------------
>
> (Updated Feb. 4, 2014, 4:49 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
>
>
> Bugs: aurora-128
> https://issues.apache.org/jira/browse/aurora-128
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Add an exception catch to provide a better error message when distributed command runner fails due to un-specified executor parameters in the slave configurations.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/command_runner.py c645e9556b50b332cc5a298ceee3d9188a68a727
> src/test/python/apache/aurora/client/commands/test_run.py abcc275d57373038227d3805c16b17b967f92048
>
> Diff: https://reviews.apache.org/r/17724/diff/
>
>
> Testing
> -------
>
> Manual testing on the command line.
>
>
> Thanks,
>
> Mark Chu-Carroll
>
>
Re: Review Request 17724: Add an exception catch to provide a better error
message when distributed command runner fails
Posted by Jonathan Boulle <jo...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17724/#review33834
-----------------------------------------------------------
Ship it!
- Jonathan Boulle
On Feb. 4, 2014, 9:49 p.m., Mark Chu-Carroll wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17724/
> -----------------------------------------------------------
>
> (Updated Feb. 4, 2014, 9:49 p.m.)
>
>
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
>
>
> Bugs: aurora-128
> https://issues.apache.org/jira/browse/aurora-128
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Add an exception catch to provide a better error message when distributed command runner fails due to un-specified executor parameters in the slave configurations.
>
>
> Diffs
> -----
>
> src/main/python/apache/aurora/client/api/command_runner.py c645e9556b50b332cc5a298ceee3d9188a68a727
> src/test/python/apache/aurora/client/commands/test_run.py abcc275d57373038227d3805c16b17b967f92048
>
> Diff: https://reviews.apache.org/r/17724/diff/
>
>
> Testing
> -------
>
> Manual testing on the command line.
>
>
> Thanks,
>
> Mark Chu-Carroll
>
>