New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Check if attributes are present in new_state before accessing them #71967
Check if attributes are present in new_state before accessing them #71967
Conversation
Hey there @dgomes, mind taking a look at this pull request as it has been labeled with an integration ( |
if self._unit_of_measurement != new_unit_of_measurement: | ||
self._unit_of_measurement = new_unit_of_measurement | ||
update_state = True | ||
if new_state.attributes: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will not cover the issue in #71684
AttributeError: 'NoneType' object has no attribute 'attributes'
Something like:
if new_state and ATTR_UNIT_OF_MEASUREMENT in new_state.attributes:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah rats, you're right. I was under the impression that the lack of 'attributes' being present in the state was causing this issue.
In that case I think it's best to pull the None / Unknown handling of new_state that's already in there to the top, since anything else before the current location of that early return does depend on it.
@dgomes, should the workflow be run again to be able to complete this merge? |
@RoboMagus The change itself looks good, but can you please update the test case Instead of waiting for CI, you can run the test and generate a coverage report locally: pytest --cov=homeassistant/components/integration --cov-report html -- tests/components/integration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make sure the modified code is covered by tests
Thanks @emontnemery, I had already checked if no tests were broken, but was unaware of the drop in coverage (and the target for it). |
Proposed change
Not every update that causes
calc_integration
to run is guaranteed to contain attributes. Example: when reloading template sensors, they may briefly exist in an unknown state.This change first checks if attributes are present in the new state, before attempting to access them.
Fixes #71684.
Type of change
Additional information
Checklist
black --fast homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
..coveragerc
.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: