FR: add "import_executor": true to manifest

[Mariusthvdb]

please see https://discord.com/channels/330944238910963714/427516175237382144/1212682202433716264

moving possible event loop blocking components BDraco suggests adding the above line to the manifest file.

see: https://github.com/home-assistant/core/pull/111772/files#diff-bf8da1d2c4d5a69bf051bd6087b795bdf1313b0b8fddbaae567814d271bb8256R21

I didnt yet test it locally, as I am not sure it would be the only thing required for it to be functional.

Please have a look?

another suggestion by Nick is:

instead of yield I think the work could be done in the executor https://github.com/pnbruckner/ha-entity-tz/pull/5/files no more event loop blocking

I copied this without understanding it, so hope you do.... thanks

[pnbruckner]

I tried running the integration w/ 2024.3.0b1 and enabling debug for homeassistant.loader. I did not see any times that I thought were excessive. I'm not sure where the problem is, or even what the problem is.

But it does look like TimezoneFinder.timezone_at can do file I/O, so yes, it probably does need to be run in an executor.

[Mariusthvdb]

It is just that we were identifying components that took longer times to initialize (longer than 0.0x ) seconds, and entity-tz rated

Component entity_tz import took 0.150 seconds (loaded_executor=False)

this was reason for core dev to mark it for improvement, and possibly the new fix as introduced in HA 2024.3 we currently run I beta

btw I did try to add that single line now to the manifest, but it still returns False during startup, so that wasnt enough on its own.

the timing might seem futile, but compared to most regular components and even your own composite:

Component composite import took 0.002 seconds (loaded_executor=False)

and the numbers start talking

[pnbruckner]

FWIW, I'm seeing:

2024-02-29 09:47:40.236 DEBUG (MainThread) [homeassistant.loader] Component entity_tz import took 0.058 seconds (loaded_executor=False)
2024-02-29 09:47:42.324 INFO (MainThread) [homeassistant.setup] Setup of domain entity_tz took 2.1 seconds
2024-02-29 09:47:42.333 DEBUG (MainThread) [homeassistant.loader] Importing platform entity_tz.config_flow took 0.01s (loaded_executor=False)
2024-02-29 09:47:42.453 DEBUG (MainThread) [homeassistant.bootstrap] Integration setup times: {..., 'entity_tz': 2.2846958520012777, ...}

I just changed the code to call TimezoneFinder.timezone_at in an executor. I'll also see what changing manifest.json does, if anything.

[pnbruckner]

After setting "import_executor": true:

2024-02-29 10:05:44.880 DEBUG (MainThread) [homeassistant.loader] Component entity_tz import took 0.474 seconds (loaded_executor=True)
2024-02-29 10:05:46.427 INFO (MainThread) [homeassistant.setup] Setup of domain entity_tz took 1.5 seconds
2024-02-29 10:05:46.432 DEBUG (MainThread) [homeassistant.loader] Importing platform entity_tz.config_flow took 0.00s (loaded_executor=False)
2024-02-29 10:05:46.558 DEBUG (MainThread) [homeassistant.bootstrap] Integration setup times: {..., 'entity_tz': 1.7573324230070284, ...}

My biggest concern with this is, what will happen with older versions of HA? I'll need to test before I include this in a release, since I want to support older HA versions like I've always done.

[Mariusthvdb]

wait, I restarted HA b1 and now see:

2024-02-29 16:57:05.034 DEBUG (MainThread) [homeassistant.loader] Component entity_tz import took 2.354 seconds (loaded_executor=True)
2024-02-29 16:57:05.035 INFO (MainThread) [homeassistant.setup] Setting up entity_tz

which indicates the import did work already after all.

but, changing from Yield to executor was preferred by Nick anyways, so probably better to focus on that.

Backwards compatibility: we really should ask Nick there, as I believe I saw him mentioning the import executor didnt cause trouble on older versions. Not sure about those specifics though.

https://discord.com/channels/330944238910963714/427516175237382144/1212683888594386954

[pnbruckner]

Both changes (code & manifest) work with 2024.2.5. I was actually more concerned if hassfest would complain, but it doesn't seem to.

[pnbruckner]

Can you test the changes and see if they satisfy everyone's concerns?

[Mariusthvdb]

Thanks, I will when I get back Monday. I rather not touch the instance when away from home…

[Mariusthvdb]

https://discord.com/channels/330944238910963714/427516175237382144/1214122281710784572

looks like it's fixed

thanks!