----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/61172/#review195718 -----------------------------------------------------------
src/python/lib/mesos/exceptions.py Lines 27-33 (patched) <https://reviews.apache.org/r/61172/#comment275006> I would prefer to do this at each call site instead of wrapping it up and hiding it in here. This will keep it consistent with the rest of the code base. That said, I do like keeping track of the original exception. So something like the following would be better in my opinion: ``` def __init__(self, message=None, original_exception=None): self.original_exception = original_exception super(MesosException, self).__init__(message) ``` Is the name `original_exception` the best name here? Is there a more standard way of wrapping exceptions like this in python and keeping track of the original ones? src/python/lib/mesos/exceptions.py Line 50 (original), 61 (patched) <https://reviews.apache.org/r/61172/#comment275007> Can you add the text `The url '{url}' ...` to the beginning of the string so that it composes well with other exceptions. They should all begin with a capital letter. src/python/lib/mesos/http.py Lines 201-205 (original), 218-221 (patched) <https://reviews.apache.org/r/61172/#comment275008> Can we simplify this to: ``` if know_exception: raise known_exception(response) else: raise MesosHTTPException(response) ``` src/python/lib/mesos/http.py Line 278 (original), 290 (patched) <https://reviews.apache.org/r/61172/#comment275009> Error strings should beign with a capital letter. src/python/lib/mesos/http.py Line 322 (original), 333 (patched) <https://reviews.apache.org/r/61172/#comment275010> Error strings should beign with a capital letter. src/python/lib/tests/test_http.py Line 283 (original), 286 (patched) <https://reviews.apache.org/r/61172/#comment275011> The tmeout parameter should be moved to the next line - Kevin Klues On Jan. 18, 2018, 4:25 a.m., Eric Chung wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/61172/ > ----------------------------------------------------------- > > (Updated Jan. 18, 2018, 4:25 a.m.) > > > Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues. > > > Repository: mesos > > > Description > ------- > > Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions > modules, which provides a Resource class and its descendants for > abstracting away common operations over http connectioins with JSON > serialization. > > updated patch according to klueska's recommendations > > > Diffs > ----- > > src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 > src/python/lib/mesos/exceptions.py PRE-CREATION > src/python/lib/mesos/http.py PRE-CREATION > src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 > src/python/lib/tests/conftest.py PRE-CREATION > src/python/lib/tests/test_exceptions.py PRE-CREATION > src/python/lib/tests/test_http.py PRE-CREATION > src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 > support/pylint.config f74f553e238553bd6a6c06f4dd888cc5954a33eb > > > Diff: https://reviews.apache.org/r/61172/diff/9/ > > > Testing > ------- > > install tox > cd src/python/lib > tox > > > Thanks, > > Eric Chung > >
