Skip to content
This repository has been archived by the owner on Mar 15, 2021. It is now read-only.

Error codes on NSX (e.g. 400, 500) should throw an exception instead of SystemExit #13

Closed
yfauser opened this issue Sep 19, 2016 · 5 comments

Comments

@yfauser
Copy link
Contributor

yfauser commented Sep 19, 2016

Currently nsxramlclient wil use SystemExit if a non successful status code like 400 or 500 is received from NSX (actually everything that is not in 200, 201 or 204).
Instead of using SystemExit we should define an exception and pass the error received from NSX into the Exception.

In the mean time one can catch non successful exits by using the following try/except block:

try:
       ------
       -----
except SystemExit as e:

      -----

@Asayu123
Copy link

Hi, I'm a user of your nice library and I was just about to post the same problem.(with version 2.0.3)

In my opinion, raise Error is one of nice option, but another solution is to simply return OrderedDict same as the success response in http_session.py .

It can be achieved by just escaping line-109 with small fix in line-103 (by calling 'xml_to_dict(et.fromstring(response.content))' instead of 'pretty_xml(response.content)').

Then, user can handle http error by referring OrderedDict's 'status'.

If you prefer, i can send pull request that modify error handling like above.
I'm looking forward your response. Thank you in advance.

@yfauser
Copy link
Contributor Author

yfauser commented Nov 2, 2016

Hi @Asayu123 .... sorry that I'm late replying. I was really busy traveling. and now I have a pile of things to work on. I like your idea! The only problem I see is the dependence on exiting for existing code (e.g. nsxansible, pynsxv, ...).
Can I suggest an option when Instantiating the client Object? With a default being sysexit, and other options being raise and continue (and pass the HTTP status code as you suggest).
What do you think?

@Asayu123
Copy link

Asayu123 commented Nov 3, 2016

Hi @yfauser , I am glad you are safe!

Can I suggest an option when Instantiating the client Object? With a default being sysexit, and other options being raise and continue (and pass the HTTP status code as you suggest).

I think it's a great idea because it gives both compatibility and flexibility. I like it !

yfauser pushed a commit that referenced this issue Dec 2, 2016
- Updated readme with the new fail_mode= parameter and version bump
yfauser pushed a commit that referenced this issue Dec 2, 2016
- Packaged nsxramlclient version 2.0.4
@yfauser
Copy link
Contributor Author

yfauser commented Dec 2, 2016

Hi @Asayu123, I added the 'raise' and 'continue' option to nsxramlclient into the devel branch. You can look at the last 3 commits referencing this issue for details.

Can you test in your environment, and see if the new code works for you and is according to your expectation?

If it works for you I will merge the new Version into master.

You can download the 2.0.4 tarbal from the /dist folder and install it using 'pip install nsxramlclient....tar.gz'

Thanks!

yfauser pushed a commit that referenced this issue Dec 3, 2016
- Added __str__ of the NsxError exception, so that one can simply use 'print e'
yfauser pushed a commit that referenced this issue Dec 3, 2016
- Added changed 2.0.4 tarball for externel testing
@Asayu123
Copy link

Asayu123 commented Dec 5, 2016

Hi @yfauser , I tested your commit and I found it works perfect for me !

I believe that both new option will make this library much easier to handle.
Thank you for taking time out of your busy schedule.

@yfauser yfauser closed this as completed in ca2ae67 Dec 5, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants