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 20521: Extend the client configuration plugin architecture.
Date Mon, 21 Apr 2014 17:40:42 GMT


> On April 21, 2014, 4:53 p.m., Mark Chu-Carroll wrote:
> > src/main/python/apache/aurora/client/cli/__init__.py, line 337
> > <https://reviews.apache.org/r/20521/diff/1/?file=563067#file563067line337>
> >
> >     The pre-execution plugin takes the same command-context as the actual verb implementation.
It behaves exactly as if it's part of the command implementation - same context, same error-signalling
methods.
> >     
> >     The post-execution plugin method shouldn't be throwing exceptions at all. It's
just a cleanup - that's why it isn't supposed to change the return code.
> >     
> >     Python being python, we can't enforce the fact that the post-execution method
doesn't throw context.CommandException. But the intention is as it's written in the code:
no attempt to catch any exception from it, because it's not supposed to throw any.
> >     
> >     Since it's not clear, I'll update the code documentation to reflect this.
> >
> 
> Maxim Khutornenko wrote:
>     Perhaps I missing something but is there anything preventing all plugins from raising
a ConfigurationPlugin.Error instead of CommandError on hook validation failure? This would
remove ambiguity on the error handling side and enable a plugin-specific error messaging/handling.
> 
> Mark Chu-Carroll wrote:
>     Preventing it? No.
>     
>     I just think that as a design point, it's clearer to say that the pre-execution method
of a plugin runs as a part of the verb implementation, and so it signals errors in the same
way. 
>     
>     The only reason that there's a plugin exception type at all is because the pre-dispatch
method doesn't have a context object, and you need a context to throw a Context.CommandError.
>

"...runs as a part of the verb implementation..." - completely agree. 
"...signals errors in the same way..." - not sure what benefits this design choice actually
brings. On the downside though, it makes it much harder to treat hook errors in a special
way if needed after different causes are multiplexed and the original context is lost. It's
always easier to treat two different errors the same way though if custom processing is not
needed (yet).

I am curious what others think here.


- Maxim


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


On April 21, 2014, 4:53 p.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20521/
> -----------------------------------------------------------
> 
> (Updated April 21, 2014, 4:53 p.m.)
> 
> 
> Review request for Aurora, David Robinson and Maxim Khutornenko.
> 
> 
> Bugs: aurora-332
>     https://issues.apache.org/jira/browse/aurora-332
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Extend the client configuration plugin architecture.
> 
> Plugins can now run code at three key points:
> - Before arguments are processed and execution is dispatched to a command.
> - After argument processing and dispatch, but before execution.
> - After execution.
> 
> This allows plugins to perform initialization required for argument processing,
> and for post-execution cleanups and synchronizations.
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/cli/__init__.py f1165a05c7c66d7a06f4733eb65ae4a7de6fad76

>   src/test/python/apache/aurora/client/cli/test_plugins.py 7cacb02ffda05fdf87a9334ed1de2092efb39f8b

> 
> Diff: https://reviews.apache.org/r/20521/diff/
> 
> 
> Testing
> -------
> 
> [sun-wukong incubator-aurora (plugin_with_cleanup)]$ ./pants src/test/python/apache/aurora/client/cli:all
> Build operating on targets: OrderedSet([PythonTestSuite(src/test/python/apache/aurora/client/cli/BUILD:all)])
> ============================================== test session starts ===============================================
> platform darwin -- Python 2.6.8 -- py-1.4.20 -- pytest-2.5.2
> collected 4 items
> 
> src/test/python/apache/aurora/client/cli/test_bridge.py ....
> 
> ============================================ 4 passed in 0.02 seconds ============================================
> ============================================== test session starts ===============================================
> platform darwin -- Python 2.6.8 -- py-1.4.20 -- pytest-2.5.2
> collected 5 items
> 
> src/test/python/apache/aurora/client/cli/test_help.py .....
> 
> ============================================ 5 passed in 0.76 seconds ============================================
> ============================================== test session starts ===============================================
> platform darwin -- Python 2.6.8 -- py-1.4.20 -- pytest-2.5.2
> collected 36 items
> 
> src/test/python/apache/aurora/client/cli/test_cancel_update.py ..
> src/test/python/apache/aurora/client/cli/test_create.py ....
> src/test/python/apache/aurora/client/cli/test_diff.py ...
> src/test/python/apache/aurora/client/cli/test_kill.py .........
> src/test/python/apache/aurora/client/cli/test_open.py .....
> src/test/python/apache/aurora/client/cli/test_restart.py ...
> src/test/python/apache/aurora/client/cli/test_status.py .......
> src/test/python/apache/aurora/client/cli/test_update.py ...
> 
> =========================================== 36 passed in 2.02 seconds ============================================
> ============================================== test session starts ===============================================
> platform darwin -- Python 2.6.8 -- py-1.4.20 -- pytest-2.5.2
> collected 1 items
> 
> src/test/python/apache/aurora/client/cli/test_logging.py .
> 
> ============================================ 1 passed in 0.68 seconds ============================================
> ============================================== test session starts ===============================================
> platform darwin -- Python 2.6.8 -- py-1.4.20 -- pytest-2.5.2
> collected 2 items
> 
> src/test/python/apache/aurora/client/cli/test_plugins.py ..
> 
> ============================================ 2 passed in 0.63 seconds ============================================
> ============================================== test session starts ===============================================
> platform darwin -- Python 2.6.8 -- py-1.4.20 -- pytest-2.5.2
> collected 4 items
> 
> src/test/python/apache/aurora/client/cli/test_quota.py ....
> 
> ============================================ 4 passed in 0.61 seconds ============================================
> ============================================== test session starts ===============================================
> platform darwin -- Python 2.6.8 -- py-1.4.20 -- pytest-2.5.2
> collected 5 items
> 
> src/test/python/apache/aurora/client/cli/test_sla.py .....
> 
> ============================================ 5 passed in 0.59 seconds ============================================
> ============================================== test session starts ===============================================
> platform darwin -- Python 2.6.8 -- py-1.4.20 -- pytest-2.5.2
> collected 2 items
> 
> src/test/python/apache/aurora/client/cli/test_task_run.py ..
> 
> ============================================ 2 passed in 0.59 seconds ============================================
> src.test.python.apache.aurora.client.cli.bridge                                 .....
  SUCCESS
> src.test.python.apache.aurora.client.cli.help                                   .....
  SUCCESS
> src.test.python.apache.aurora.client.cli.job                                    .....
  SUCCESS
> src.test.python.apache.aurora.client.cli.logging                                .....
  SUCCESS
> src.test.python.apache.aurora.client.cli.plugins                                .....
  SUCCESS
> src.test.python.apache.aurora.client.cli.quota                                  .....
  SUCCESS
> src.test.python.apache.aurora.client.cli.sla                                    .....
  SUCCESS
> src.test.python.apache.aurora.client.cli.task                                   .....
  SUCCESS
> [sun-wukong incubator-aurora (plugin_with_cleanup)]$
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>


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