request.close or AsyncClient.close? #342

Activity

1. florimondmanca

   added

   questionFurther information is requestedFurther information is requested

   on Sep 14, 2019
2. florimondmanca commented on Sep 14, 2019

   

   florimondmanca

   on Sep 14, 2019

   Last edited by florimondmanca

   Contributor

   More actions

   Hi @edocod1,

   I'm assuming you are invoking the client as shown in the docs, i.e.:

   client = httpx.AsyncClient()
   response = await client.get(...)

   Switching to the context manager syntax will close all remaining keep-alive connections on exit:

   async with httpx.AsyncClient() as client:
       response = await client.get(...)
   
   # All connections are now closed.
   # You can still safely access the read response content outside the context block, e.g.:
   html = response.text

   If you're using stream=True then you need to make sure to .close() the response at some point, as documented here. We have an open issue to close pending streaming responses on exit too (#35) which would allow users not to worry about manually closing the response.

   Does this help? Feel free to share some code if you'd like help on some more specific items.
3. edocod1 commented on Sep 15, 2019

   

   edocod1

   on Sep 15, 2019

   Author

   More actions

   Oh so the AsyncClient has a context manager syntax...
   I'm gonna try switching to that, to see if I keep leaking connections. Additionally, consider i have a list like this:

   links = ['//site.com/hello.jpg', '//site.com/bye.jpg', '//different.yes/another.jpg', '//site.com/ofcourse.jpg, '//site.com/yes.jpg']

   and i ran it in a loop:

      for url in links:
         r = await client.get(url)
         await response.read()
         # Other stuff here
   

   To my understanding it would do something like this:

   • open connection to site.com, request hello.jpg and bye.jpg
   • open connection to different.yes, request another.jpg
   • reuse connection to site.com, to request ofcourse.jpg and yes.jpg
   • close both connections (after exiting the with block)
     Right? Or would the connection to different.yes drop the one on site.com?

   Additionally, I've extended the AsyncClient class to retry in case of failure:

   TRY_DELAY = 5
   DELAY_INCR = 5
   TIMEOUT = 20
   
   class SafeAsyncClient(httpx.AsyncClient):
   	log = logging.getLogger('safe_httpx')
   
   	async def get(*args, **kwargs):
   		kwargs['timeout'] = kwargs.get('timeout', TIMEOUT)
   
   		sleep_delay = TRY_DELAY
   		exc = None
   
   		for try_ in range(TRIES):
   
   			if try_ == TRIES-1:
   				AsyncClient.log.error(f"Request ({args[1][:64]}) last failure ({why}).")
   				if exc:
   					raise exc
   				else:
   					return request
   			try:
   				request = await httpx.AsyncClient.get(*args, **kwargs)
   			except (Timeout, RedirectError, ConnectionRefusedError) as e:
   				why = type(e).__name__
   				exc = e
   			except:
   				raise
   			else:
   				if request.status_code in (408, 429, 500, 502, 503, 504, 512, 522, 523, 524, 526, 530):
   					why = f"Got status code {request.status_code} {request.reason_phrase}"
   				else:
   					return request
   
   			AsyncClient.log.warning(f"Request ({args[1][:64]}) failure ({why}). Try {try_+1}/{TRIES}")
   			AsyncClient.log.info(f"Sleeping {TRY_DELAY}s before retrying")
   			await sleep(sleep_delay)
   			sleep_delay += DELAY_INCR
   

   Would this mess with the connection closing mechanism?
4. florimondmanca commented on Sep 15, 2019

   

   florimondmanca

   on Sep 15, 2019

   Last edited by florimondmanca

   Contributor

   More actions

   To my understanding it would do something like this: …

   I think you're correct. A keep-alive connection to one site isn't dropped when you request another one (that's actually the purpose of the internal connection pool).

   About the retry extension, the .get() method is one level of abstraction above the connection pool, so connections should all be automatically managed and cleaned up as usual. (A note on the code: you should probably be using self.log instead of AsyncClient.log and super().get() instead of httpx.AsyncClient.get().)

   I guess your approach works but it shows the necessity of providing a clean extension mechanism for Client/AsyncClient. :-) (Actually, we have an internal middleware mechanism to handle this kind of use case, but it's not public yet, i.e. there's no way that add another middleware to the client from the outside yet. You can take a look at #295 for discussions.)
5. florimondmanca commented on Sep 15, 2019

   

   florimondmanca

   on Sep 15, 2019

   Contributor

   More actions

   @edocod1 It turns out I was wrong about the necessity of calling response.read(). Actually, you only need to do so if you're using response streaming, e.g. await client.get(..., stream=True). I'll update my comment with a fix.
6. florimondmanca commented on Sep 15, 2019

   

   florimondmanca

   on Sep 15, 2019

   Last edited by florimondmanca

   Contributor

   More actions

   @edocod1 Feel free to take a look at and comment on #344 which proposes a documentation update. :)
7. florimondmanca

   closed this as completedin #344on Sep 15, 2019
8. edocod1 commented on Sep 15, 2019

   

   edocod1

   on Sep 15, 2019

   Author

   More actions

   Thank you very much for the help and suggestions! I was able to fix it.
   Have a nice day

   👍React with 👍1Reacted by Florimond Manca