----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62938/#review190157 -----------------------------------------------------------
src/python/cli_new/lib/cli/util.py Line 225 (original), 227 (patched) <https://reviews.apache.org/r/62938/#comment267409> Can you please remove the second sentence here. It seems unnecessary. src/python/cli_new/lib/cli/util.py Lines 233 (patched) <https://reviews.apache.org/r/62938/#comment267408> We don't need this anymore. src/python/cli_new/lib/cli/util.py Lines 235 (patched) <https://reviews.apache.org/r/62938/#comment267410> Do we need a try/except block here? We can wrap the whole Kazoo initialization in a single one and print the corresponding error. src/python/cli_new/lib/cli/util.py Lines 240-245 (patched) <https://reviews.apache.org/r/62938/#comment267417> We usually just catch an "Exception" and then let the error string indicate to us what went wrong. src/python/cli_new/lib/cli/util.py Lines 246 (patched) <https://reviews.apache.org/r/62938/#comment267411> We should probably have a catchall "Exception" caught here in case we throw an exception for any other reason. src/python/cli_new/lib/cli/util.py Lines 252 (patched) <https://reviews.apache.org/r/62938/#comment267413> I feel like we need a try catch around this whole thing, with the error prefix string saying something like: "Unable to communicate with mesos master using ZK: {error}" src/python/cli_new/lib/cli/util.py Lines 253 (patched) <https://reviews.apache.org/r/62938/#comment267416> This could throw an exception that we need to catch. src/python/cli_new/lib/cli/util.py Lines 255 (patched) <https://reviews.apache.org/r/62938/#comment267414> This could throw an exception that we need to catch. src/python/cli_new/lib/cli/util.py Lines 258 (patched) <https://reviews.apache.org/r/62938/#comment267415> This could throw an exception that we need to catch. - Kevin Klues On Oct. 26, 2017, 4:52 p.m., Armand Grillet wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62938/ > ----------------------------------------------------------- > > (Updated Oct. 26, 2017, 4:52 p.m.) > > > Review request for mesos, Eric Chung and Kevin Klues. > > > Bugs: MESOS-8012 > https://issues.apache.org/jira/browse/MESOS-8012 > > > Repository: mesos > > > Description > ------- > > This change allows users to use the CLI with a Mesos running in high > availability mode. The `zookeeper` field was already here before this > commit, with an `addresses` array and a `path` field. This change > only adds the backend to actually make it usable. > > Interacting with ZooKeeper requires a new dependency, kazoo, that has > been added to the list of requirements for the CLI. > > > Diffs > ----- > > src/python/cli_new/README.md a4b270d9b54cdd83bd62530fb44ddfaffb8a014b > src/python/cli_new/lib/cli/util.py dd109c09368e650b7d7dabd663c11bc2e3e5180a > src/python/cli_new/pip-requirements.txt > 7aeac344c47ccd2588fded44d7314db7abd85653 > > > Diff: https://reviews.apache.org/r/62938/diff/3/ > > > Testing > ------- > > Tested with a Mesos cluster running with 3 masters and 1 agent, run some CLI > commands successfully. > > > Thanks, > > Armand Grillet > >
