jrxFive / python-nomad

Client library Hashicorp Nomad
https://python-nomad.readthedocs.io/en/latest/
MIT License
139 stars 73 forks source link

Exception Handling #47

Closed etrabelsi closed 6 years ago

etrabelsi commented 6 years ago

Hi, When a request raise an error , the message is "gone" due to try except blocks on post/get function. If i create PR will this something you are willing to merge? @jrxFive

jrxFive commented 6 years ago

Hey etrabelsi,

Sorry to hear that. Absolutely would welcome a PR.

Thanks

jrxFive commented 6 years ago

Took a peek, yea totally should have had used msg and message instead of nomad_resp. Is nomad_resp not an instance value when raised? It would be a request response object.

etrabelsi commented 6 years ago

@jrxFive to be honest i dont understand _"Is nomadresp not an instance value when raised? It would be a request response object."

Basically What I suggest is replacing:

   try:
        url = self._requester._endpointBuilder(<resource>.ENDPOINT, *args)
        response = self._requester.post(url, json=kwargs.get("json_dict", None))
        return response.json()
   except:
        raise

with

   try:
        url = self._requester._endpointBuilder(<resource>.ENDPOINT, *args)
        response = self._requester.post(url, json=kwargs.get("json_dict", None))
        return response.json()
   except Exception as e:
        raise BaseNomadException(e.message)

and to change BaseNomadException to get message. legit?

jonathanrcross commented 6 years ago

Yup that would work, with the minor change of it not being e.message but with e.nomad_resp. Since I did not use message python3+ nor msg python2 .

We could probably make it a bit more sophisticated by checking the instanceof e with the custom exception classes and raising the proper one back.

jrxFive commented 6 years ago

@etrabelsi, also we could probably just remove this try catches that should raise the exception with the proper object and response object.

etrabelsi commented 6 years ago

@jrxFive did you mean something like this https://github.com/jrxFive/python-nomad/pull/48

jrxFive commented 6 years ago

Resolved in #48