mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Eric Chung <cinchu...@gmail.com>
Subject Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.
Date Fri, 19 Jan 2018 20:48:43 GMT


> On Jan. 18, 2018, 1:59 p.m., Kevin Klues wrote:
> > src/python/lib/mesos/exceptions.py
> > Lines 27-33 (patched)
> > <https://reviews.apache.org/r/61172/diff/7-9/?file=1904480#file1904480line27>
> >
> >     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?

it seems there is no good solution for this use case in python 2, however it is supported
natively in python 3 via exception chaining: https://www.python.org/dev/peps/pep-3134/ maybe
another incentive for us to migrate. :)

just curious though, are you saying i shouldn't append the exception to the message?


- Eric


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61172/#review195718
-----------------------------------------------------------


On Jan. 19, 2018, 8:48 p.m., Eric Chung wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> -----------------------------------------------------------
> 
> (Updated Jan. 19, 2018, 8:48 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added mesos.http and mesos.exceptions for CLI.
> 
> 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/10/
> 
> 
> Testing
> -------
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message