Revert "Gather update methods to update faster"

[bdraco]

Reverts #301

home-assistant/core#116755

It turns out that some of the slower devices cannot respond within 10s to all the requests because the API is too slow.

While this will make updates much slower for faster devices, we still need to support the older slower ones.

[mib1185]

can we increase the timeout to let's say 30s instead and keep the batch processing?

[bdraco]

Changing the default timeout to 30s would probably work as well, however it won't fix anyone with a slow system who has manually set the timeout lower. We could do a one time options change to increase to 30s in the integration if its less than 30s, but we usually try not to alter user's settings. However its not common or a usual design to allow for a user configurable timeout in the first place, and would likely be rejected as an option for any new integration in code review as the timeout value is something developers should pick.

[bdraco]

Also I think there is another issue going on here home-assistant/core#116755 (comment)

[mib1185]

i meant so change the timeout just for this "bulk fetch" task - something like

async with asyncio.timeout(30)
    await asyncio.gather(*update_methods)

[mib1185]

would likely be rejected as an option for any new integration in code review as the timeout value is something developers should pick.

i agree and already thought about removing both, the timeout and the scan interval option from the integration

[bdraco]

i meant so change the timeout just for this "bulk fetch" task - something like

async with asyncio.timeout(30)
    await asyncio.gather(*update_methods)

Usually we want aiohttp to handle the timeout

py-synologydsm-api/src/synology_dsm/synology_dsm.py

Line 77 in 737fef6

self._aiohttp_timeout = aiohttp.ClientTimeout(total=timeout)

since if a timeout hits inside the asyncio.timeout context manager it will cancel all the tasks inside which sometimes leads to races in tearing down the connections which can make subsequent requests fail. That really shouldn't happen but it does in practice as we have to be sure everything inside an asyncio.timeout block is always cancellation safe. In theory aiohttp should be cancellation safe but we still see bug reports in the aiohttp issue queue where its not.

Another option might be to use gather_with_limited_concurrency https://github.com/home-assistant/core/blob/985fd499094297fc352e0c530e99264c0b742686/homeassistant/util/async_.py#L96 to reduce the number of requests.

I think before we know what to do we need to see if increasing the timeout works for the user

[bdraco]

Let's hold off on this until we know if increasing the timeout to 30s solves the issue home-assistant/core#116755 (comment)

[mib1185]

I think increasing the timeout to 30s should be sufficient - we have a quiet old and slow ds213+ home-assistant/core#116755 (comment) which is fine with 30s

[bdraco]

Let's go with adjusting the default value in HA instead.