search

ibmresilient / resilient-python-api

Python Library for the IBM SOAR REST API, a Python SDK for developing Apps for IBM SOAR and more...
https://ibm.biz/soar-python-docs
MIT License
39 stars 28 forks source link

`RetryHTTPException` Should Inherit from `SimpleHTTPException ` #32

Closed lmahoney1 closed 1 year ago

lmahoney1 commented 1 year ago

Previously when using the self.rest_client() object within an IBM SOAR function it would raise a SimpleHTTPException exception when it encountered a web request error. Must have been a recent change but I'm seeing that certain response codes are getting re-tried and if all retries fail a RetryHTTPException is raised.

I think this is great, however all of my IBM SOAR functions I have created are setup to catch SimpleHTTPExceptions and not RetryHTTPException.

If RetryHTTPException inherited from SimpleHTTPException it would still get caught by

except SimpleHTTPException as e:
   log.info(<information about request / response>)

Not 100% positive that SimpleHTTPException would be the best 'root' / generic web request to inherit from, but it would work best for me.

I can change my except blocks to be 

except (SimpleHTTPException, RetryHTTPException) as e:
   ...

but I would rather not have to change that in all of our functions. I think it makes logical sense for RetryHTTPException to inherit from a 'generic' http exception, just not 100% positive if SimpleHTTPException fits that bill or not.

Additionally, importing these two exceptions is different:

from resilient import SimpleHTTPException
from resilient.co3base import RetryHTTPException

Noticed that SimpleHTTPException is imported in /resilient/__init__.py but RetryHTTPException isn't. I think a consistent import would be nice - so something like from resilient import SimpleHTTPException, RetryHTTPException would work

RetryHTTPException definition: https://github.com/ibmresilient/resilient-python-api/blob/84e8c6d9140ceac0bf47ce0b98e11c7953d95e61/resilient/resilient/co3base.py#L72

bobleckelibm commented 1 year ago

@lmahoney1 this is a fantastic idea. I will begin investigation on this in the coming release timeframe and let you know whether or not we think this is possible. On first glance, I can't see why we wouldn't do this.

bobleckelibm commented 1 year ago

Hi @lmahoney1 — we've figured out the root issue here and the solution is actually to include resilient.co3base.BasicHTTPException as the parent class for the new RetryHTTPException. This is because all resilient clients are SimpleClients and the SimpleClient is where the SimpleHTTPException is raised. This change to the base class of RetryHTTPException will fix that up and exceptions will begin to be raised as SimpleHTTPExceptions again.

We plan to make this change as soon as we can and get it tested and released. It will not make the version 50 release which will come before the end of the month. But it will be available soon after that and I'll let you know how that goes.

bobleckelibm commented 1 year ago

Hi @lmahoney1 — we've developed a fix for this and it will be targeted for a 50.1.x version release later this month or early next. I'm going to close out this issue now given that the fix is already in QA. Thanks!

bobleckelibm commented 1 year ago

@lmahoney1 released today to PyPi! v50.1.262

lmahoney1 commented 1 year ago

Great news. Thanks for the work on this Bo, much appreciated!