----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20928/#review42283 -----------------------------------------------------------
src/main/python/apache/aurora/client/cli/__init__.py <https://reviews.apache.org/r/20928/#comment76059> Done. (Getting comments like this in a review is always a bit frustrating, because I just want to get stuff done. But then, I look at the code, and you're absolutely right, and the code is so much nicer after I fix it. This is exactly what makes code review so valuable. Thanks!) src/main/python/apache/aurora/client/cli/__init__.py <https://reviews.apache.org/r/20928/#comment76060> I don't think so. I thought it was just 4 space indent on the next line. That's the style that I've used throughout the client. src/main/python/apache/aurora/client/cli/command_hooks.py <https://reviews.apache.org/r/20928/#comment76064> In this case, these submethods are just there to make the condition below more readable. They're not intended to ever be used outside of this method (which is why they're privates). The overall method that they're part of has a good set of tests. src/main/python/apache/aurora/client/cli/command_hooks.py <https://reviews.apache.org/r/20928/#comment76066> (I could have sworn I replied to this already, but reviewboard says I didn't. Forgive me if this is a duplicate.) These are local private methods which only exist to make the compound conditional below easier to read. They should never (and in fact can never) be called from anywhere else. The condition that they're part of has a decent set of tests. src/main/python/apache/aurora/client/cli/command_hooks.py <https://reviews.apache.org/r/20928/#comment76067> Added an additional link at the top of the file, but I'd prefer to also leave this here. src/main/python/apache/aurora/client/cli/command_hooks.py <https://reviews.apache.org/r/20928/#comment76065> Nope, this is the correct behavior. The client should not abort because it couldn't read skip rules. The only effect that the skip rules error will have is that attempts to execute commands blocked by hooks won't be executable. We don't want the client to abort all commands because some commands might be block. All we want to do is let them (and the loggie service) know that there was a problem with the skip rules. This will get logged to loggie, because it's got a global handler in place. - Mark Chu-Carroll On May 1, 2014, 5:39 p.m., Mark Chu-Carroll wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/20928/ > ----------------------------------------------------------- > > (Updated May 1, 2014, 5:39 p.m.) > > > Review request for Aurora, David McLaughlin and Suman Karumuri. > > > Bugs: aurora-270 > https://issues.apache.org/jira/browse/aurora-270 > > > Repository: aurora > > > Description > ------- > > The second half of command hooks: > - Dynamically registered hook exceptions are provided, by fetching a hooks > skip rules file from a > URL. > - Hooks are loaded and activated by the noun/verb framework. > - Hook selection and dispatch has been substantially updated. > > Also did some long overdue cleanup of string quoting consistency. > > > Diffs > ----- > > 3rdparty/python/BUILD 122f71db0bc6f37c19ae0f3cb2fcade8321404e6 > docs/design/command-hooks.md ee320ed3922928408d23a2dfdf3c42ef96e62ff7 > src/main/python/apache/aurora/client/cli/BUILD > 1bd565effb3dbe2aeb5d6156575e9316bd77c6a8 > src/main/python/apache/aurora/client/cli/__init__.py > 1e396b2263451b41cd223708f4cc4cdb1eeddbf0 > src/main/python/apache/aurora/client/cli/command_hooks.py > 2d200682209e1df83c03f9d515f3b118aaa85a99 > src/test/python/apache/aurora/client/cli/test_command_hooks.py > 7c6f70c7ef7534e9dd4986364331c67f6f7c36b1 > > Diff: https://reviews.apache.org/r/20928/diff/ > > > Testing > ------- > > [sun-wukong incubator-aurora (command-hooks-two)]$ !./p > ./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 > plugins: cov > 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 > plugins: cov > collected 9 items > > src/test/python/apache/aurora/client/cli/test_command_hooks.py ......... > > =========================== 9 passed in 0.80 seconds > =========================== > ============================= test session starts > ============================== > platform darwin -- Python 2.6.8 -- py-1.4.20 -- pytest-2.5.2 > plugins: cov > collected 5 items > > src/test/python/apache/aurora/client/cli/test_help.py ..... > > =========================== 5 passed in 0.70 seconds > =========================== > ============================= test session starts > ============================== > platform darwin -- Python 2.6.8 -- py-1.4.20 -- pytest-2.5.2 > plugins: cov > 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 > plugins: cov > collected 1 items > > src/test/python/apache/aurora/client/cli/test_logging.py . > > =========================== 1 passed in 0.65 seconds > =========================== > ============================= test session starts > ============================== > platform darwin -- Python 2.6.8 -- py-1.4.20 -- pytest-2.5.2 > plugins: cov > collected 3 items > > src/test/python/apache/aurora/client/cli/test_plugins.py ... > > =========================== 3 passed in 0.67 seconds > =========================== > ============================= test session starts > ============================== > platform darwin -- Python 2.6.8 -- py-1.4.20 -- pytest-2.5.2 > plugins: cov > collected 4 items > > src/test/python/apache/aurora/client/cli/test_quota.py .... > > =========================== 4 passed in 0.67 seconds > =========================== > ============================= test session starts > ============================== > platform darwin -- Python 2.6.8 -- py-1.4.20 -- pytest-2.5.2 > plugins: cov > collected 5 items > > src/test/python/apache/aurora/client/cli/test_sla.py ..... > > =========================== 5 passed in 0.72 seconds > =========================== > ============================= test session starts > ============================== > platform darwin -- Python 2.6.8 -- py-1.4.20 -- pytest-2.5.2 > plugins: cov > collected 2 items > > src/test/python/apache/aurora/client/cli/test_task_run.py .. > > =========================== 2 passed in 0.65 seconds > =========================== > src.test.python.apache.aurora.client.cli.bridge > ..... SUCCESS > src.test.python.apache.aurora.client.cli.command_hooks > ..... 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 (command-hooks-two)]$ > > > Thanks, > > Mark Chu-Carroll > >
