Refactor async_get_hass to rely on threading.local instead of a ContextVar

[jbouwh]

Proposed change

Refactor async_get_hass to rely on threading.local instead of a ContextVar

Problem

async_get_hass allows access to the global HomeAssistant instance which is stored in a ContextVar.
The context is not copied between threads, which means the context is lost when a worker thread or an executor submits a job or task to the event loop. For example, this meant async_get_hass failed when an automation was triggered by an MQTT message, because the MQTT client runs in its own worker thread.

Solution

Let the event look store the HomeAssistant instance in a threading.local instance instead.

Type of change

• Dependency upgrade
• Bugfix (non-breaking change which fixes an issue)
• New integration (thank you!)
• New feature (which adds functionality to an existing integration)
• Deprecation (breaking change to happen in the future)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests

Additional information

• This PR fixes or closes issue: fixes Error with mqtt trigger and the new responding service #95982
• This PR is related to issue:
• Link to documentation pull request:

Checklist

• The code change is tested and works locally.
• Local tests pass. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.
• I have followed the development checklist
• I have followed the perfect PR recommendations
• The code has been formatted using Black (black --fast homeassistant tests)
• Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

• The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
• Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

• I have reviewed two other open pull requests in this repository.

[home-assistant]

Hey there @emontnemery, mind taking a look at this pull request as it has been labeled with an integration (mqtt) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of mqtt can trigger bot actions by commenting:

• @home-assistant close Closes the pull request.
• @home-assistant rename Awesome new title Renames the pull request.
• @home-assistant reopen Reopen the pull request.
• @home-assistant unassign mqtt Removes the current integration label and assignees on the pull request, add the integration domain after the command.

[emontnemery]

This can't possibly be the responsibility of MQTT.
I think we should instead do this in the non-async methods on the hass object for scheduling jobs:
hass.create_task
hass.add_job

[jbouwh]

This can't possibly be the responsibility of MQTT. I think we should instead do this in the non-async methods on the hass object for scheduling jobs: hass.create_task hass.add_job

All their methods finally call async_create_task, here the hass context var is inserted to make sure the context is correct.

[MartinHjelmare]

Do the new tests fail for the old code?

[MartinHjelmare]

Would be nice to have a PR with a commit adding the new tests first, failing them. Then update the code under test in a second commit and see the tests pass.

[jbouwh]

Would be nice to have a PR with a commit adding the new tests first, failing them. Then update the code under test in a second commit and see the tests pass.

True, but may be we need to split tests, as they can influence each other if they are in one test

[jbouwh]

Do the new tests fail for the old code?

Only the Threaded examples, so the if could be moved below and still let the test pass.

[MartinHjelmare]

Would be nice to have a PR with a commit adding the new tests first, failing them. Then update the code under test in a second commit and see the tests pass.

True, but may be we need to split tests, as they can influence each other if they are in one test

That's fine. I just meant that it would be nice to see a commit history in the PR where the first commits add the tests and the existing code under test fails the new tests. And then we update the code under test in the last commit and all the tests pass.

[jbouwh]

Would be nice to have a PR with a commit adding the new tests first, failing them. Then update the code under test in a second commit and see the tests pass.

Check is added

[jbouwh]

Check again if this solves the issue in the case with MQTT and that works okay too.

[MartinHjelmare]

When we're happy with this PR, I'd like to suggest we create a new PR with just two commits, first being new tests, second being code fix, that we can check out locally and validate with and without fix applied.

[home-assistant]

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

[emontnemery]

I think this is a more suitable solution than a context variable; the thread local data is only accessible to the event loop, and that perfectly matches the definition that async_*-functions should only ever be called from the event loop.

Thanks a lot @jbouwh 👍

[MartinHjelmare]

Great! Now I think we can rebase and make two commits so we can verify locally.

[jbouwh]

There was one bug in the test. The PR is rebased and commits for test and fix are splitted now.
Without the fix the test will fail.
The fix removes LookupError from the exception list.