search

zxdavb / evohome-async

An asyncio Python client to access the Evohome web service
http://evohome-client.readthedocs.org/en/latest/
Apache License 2.0
11 stars 11 forks source link

Loosen aiohttp requirement #11

Closed cdce8p closed 11 months ago

cdce8p commented 11 months ago

A hard pin for aiohttp will make life quite difficult for downstream packages like Home Assistant. For every new aiohttp release, we would need to wait for evohome-async to be updated first. E.g. version 3.9.0b1 was just releases which is incompatible.

Unless there is a specific feature needed, it's usually best / recommended to only pin a lower bound.

It would be great if a new release could be made after this PR is merged as this is currently blocking Home Assistant.

zxdavb commented 11 months ago

This may be better:

    "aiohttp>=3.9.0b0; python_version >= '3.12'",
    "aiohttp>=3.8.5; python_version < '3.12'",

Is that OK?

zxdavb commented 11 months ago

Yep - a stupid mistake.

cdce8p commented 11 months ago

I copy these from HA (and I think they've already changed/about to change)

HA is different as it's an application which should pin exact versions. For libraries there really is no need to do that.

but this may be better:

    "aiohttp>=3.9.0b0; python_version >= '3.12'",
    "aiohttp>=3.8.5; python_version < '3.12'",

Is that OK?

It would technically work, but isn't ideal as it would even prevent testing of 3.8.x on 3.12 if a compatible version is ever released.

cdce8p commented 11 months ago

Yep - a stupid mistake.

Not at all. What you did in requirements.txt is likely even required for CI to pass. That's where it's placed correctly. https://github.com/zxdavb/evohome-async/blob/1ba859c185e8aa82ad16da30b4694a01b467318b/requirements.txt#L3-L4

zxdavb commented 11 months ago

OK, will take me a few minutes...

zxdavb commented 11 months ago

OK, I have done this instead: https://github.com/home-assistant/core/pull/103534