search

claws / aioprometheus

A Prometheus Python client library for asyncio-based applications
176 stars 21 forks source link

Aiohttp 3+ start/shutdown changes #24

Closed hellysmile closed 6 years ago

hellysmile commented 6 years ago

Hey, aiohttp3 introduced advanced site runners https://docs.aiohttp.org/en/stable/web_advanced.html#application-runners

I am going to make PR to adjust aioprometheus to use it, but it will force to support aiohttp3+. Is it acceptable for You @claws ?

claws commented 6 years ago

Are you referring to using the aiohttp AppRunner and TCPSite objects?

I have already applied these changes as part of #23 which resolves some deprecation warnings that were being raised by the newer aiohttp.

I have not yet pushed the latest code to PyPI as a new package though.

hellysmile commented 6 years ago

Cool, I will test latest changes today, thnx

hellysmile commented 6 years ago

Master work good for me!

I have suggestion

https://github.com/claws/aioprometheus/blob/master/src/aioprometheus/service.py#L152

should have small shutdown_timeout, otherwise it can stuck for default is too big for real prometheus usage

https://github.com/aio-libs/aiohttp/blob/master/aiohttp/web_runner.py#L63

What about to make it 2 sec for example?

claws commented 6 years ago

Great.

Your suggestion regarding shutdown timeout sounds good.

hellysmile commented 6 years ago

I have small PR for this https://github.com/claws/aioprometheus/pull/25 , lets move further conversation there