Shams
Did an initial pass, looks like we should resolve any issues from https://github.com/shamsimam/savina/pull/8 before merging this. My main comments will be regarding the common benchmark runner codes, you are better...
What is a good default value? 1 week?
Has this failed for you recently? I tried the following in my repl: ``` (deftest test-prepare-sandbox-publisher (time (dotimes [_ 1000] ...))) ``` and got: ``` Running cook.test.mesos.sandbox/test-prepare-sandbox-publisher "Elapsed time: 33687.950722...
It is already enabled, maybe it got fixed during our last round of changed with the syncer.
Actually it might be a bug in my branch where even though job submission fails, an http status code of `201` is being returned.
We chatted offline and @dposada suggested there is room for improvement here by not claiming success if the UUID is not found in the response.
Got this in one of the builds: ``` [gw1] [ 81%] FAILED tests/test_executor.py::ExecutorTest::test_manage_task_terminated Replacing crashed slave gw1 https://travis-ci.org/twosigma/Cook/jobs/333407170#L658 ``` So we kind of know which test is the culprit.
I don't like the vectors in the `:catch` and `:finally` clauses. I would be happier with syntax like: ``` (try-let [x (throw (RuntimeException. "Hi")) (catch IllegalArgumentException e (comment Cannot happen))...
I think the `check-user` can become part of a `(validate-service service-id)` function in the `scheduler`.
This is already available but limited only to admins: https://github.com/twosigma/waiter/blob/8a0bd98b1cd62598aa35b308fb0776e9467c4891/cli/waiter/subcommands/ssh.py#L180-L183 The default container continues to be `waiter-app`.