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