Arvados

Reviewing c0c0d76. The code is very clean, so thank you for an easy review. I only have a couple of small points.

shuffled_service_roots has an except: clause. Can you catch a specific exception here? except: catches things like KeyboardInterrupt and SystemExit, which is not what you intend here. If you can't name the exception, at least say except Exception: to avoid that issue.

In KeepTestCase.setUpClass, would it help improve test isolation to clear the arvados module variables from setUp instead? Since you're re-doing that in specific test methods, it looks like it could simplify things a bit too. Relatedly, is it worth refactoring all the setup and teardown methods in test_keep_client to a common superclass, so you don't have to repeat it between the test cases?

1. Changed "except:" to "except Exception:". Thanks for that.
2. Refactored test_keep_client a little bit to make it clearer what is going on (and ideally a little more robust, as I realized the tests were order sensitive). It's hard to refactor very much, since tests inherently contain a lot of code that sets up an environment that's very specific to the test.

Peter Amstutz wrote:

1. Refactored test_keep_client a little bit to make it clearer what is going on (and ideally a little more robust, as I realized the tests were order sensitive). It's hard to refactor very much, since tests inherently contain a lot of code that sets up an environment that's very specific to the test.

That is helpful, thanks. This looks good to merge to me.