ssl-hep / ServiceX_frontend

Client access library for ServiceX
https://servicex-frontend.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
5 stars 11 forks source link

Restrict the retry exceptions for getting transform status in `get_tranform_status` #474

Closed ketan96-m closed 2 weeks ago

ketan96-m commented 2 months ago

Follow up on the PR: https://github.com/ssl-hep/ServiceX_frontend/pull/469 Allow only ClientError exception handling. Currently handles RuntimeError.

retry_options = ExponentialRetry(attempts=5, start_timeout=3)
async with RetryClient(retry_options=retry_options) as client:
            try:
                async for attempt in AsyncRetrying(
                        retry=(retry_if_not_exception_type(AuthorizationError)
                               | retry_if_not_exception_type(ValueError)),
                        stop=stop_after_attempt(3),
                        wait=wait_fixed(3),
                        reraise=True):

Add ClientError to retry_options

e.g log

Traceback (most recent call last):
  File "/mnt/d/utaustin/servicex/servicex_frontend/examples/func_adl_xAOD_simple.py", line 32, in <module>
    assert len(files['func_adl_xAOD_simple']) == 1
  File "/mnt/d/release/lib/python3.10/site-packages/servicex/servicex_client.py", line 87, in __len__
    raise data
servicex.servicex_client.ReturnValueException: Exception occurred while making ServiceX request.
Traceback (most recent call last):
  File "/mnt/d/release/lib/python3.10/site-packages/servicex/query_core.py", line 548, in as_files_async
    return await self.submit_and_download(
  File "/mnt/d/release/lib/python3.10/site-packages/servicex/query_core.py", line 283, in submit_and_download
    self.request_id = await self.servicex.submit_transform(sx_request)
  File "/mnt/d/release/lib/python3.10/site-packages/servicex/servicex_adapter.py", line 125, in submit_transform
    o = await r.json()
  File "/mnt/d/release/lib/python3.10/site-packages/aiohttp/client_reqrep.py", line 1199, in json
    raise ContentTypeError(
aiohttp.client_exceptions.ContentTypeError: 504, message='Attempt to decode JSON with unexpected mimetype: text/html', url='https://servicex.af.uchicago.edu/servicex/transformation'
ponyisi commented 1 month ago

I'm a bit unsure which errors you think should and shouldn't be caught here? I guess you mean that the RetryClient shouldn't try to retry anything except HTTP connection problems?

ponyisi commented 3 weeks ago

So I'm not really sure what there is to do here, and I'd be inclined to close it and move on. There is an issue I can see (and which @ketan96-m refers to in his traceback) which is that we implicitly assume that all responses from the server will be in JSON; certainly for 500-class errors this is not true, and so we will wind up raising unrelated exceptions, which have a chance of getting propagated to the user. Those could be a 3.0.2 fix.

Anyone objecting to me closing this issue, speak now...