Skip to content
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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[jinja] disable failOnUnknownTokens #16347

Merged
merged 1 commit into from Feb 4, 2024

Conversation

ccutrer
Copy link
Contributor

@ccutrer ccutrer commented Jan 30, 2024

Home Assistant doesn't enable strict mode, so we shouldn't either. Fixes Koenkk/zigbee2mqtt#17395

Home Assistant doesn't enable strict mode, so we shouldn't either

Signed-off-by: Cody Cutrer <cody@cutrer.us>
@antroids
Copy link
Contributor

It was enabled to fix the issue: #10496

Could you please check what will Python Jinja return in the case from unit test?

@ccutrer
Copy link
Contributor Author

ccutrer commented Jan 30, 2024

>>> from jinja2 import Environment
>>> env = Environment()
>>> tmpl = env.from_string("Hello {{ missing }}!")
>>> tmpl.render()
'Hello !'

@ccutrer
Copy link
Contributor Author

ccutrer commented Jan 30, 2024

Note that with #16307, the situation from the issue you referenced will end up with the item updated to NULL, which makes perfect sense for the situations that I'm seeing the UnknownTokenException (zigbee2mqtt does not know the state of something yet, and for scenes in zwave-js-ui where it goes back to not having a state immediately after triggering the scene). If you think that's not the correct behavior for a climate component, we should look at the Home Assistant source and see how it handles this.

@antroids
Copy link
Contributor

Note that with #16307, the situation from the issue you referenced will end up with the item updated to NULL, which makes perfect sense for the situations that I'm seeing the UnknownTokenException (zigbee2mqtt does not know the state of something yet, and for scenes in zwave-js-ui where it goes back to not having a state immediately after triggering the scene). If you think that's not the correct behavior for a climate component, we should look at the Home Assistant source and see how it handles this.

Yes, you are right. I'll double check the HASS sources, but looks like with #16307 this should be ok.

Copy link
Contributor

@antroids antroids left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@lolodomo lolodomo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you

@lolodomo lolodomo merged commit 24f54f5 into openhab:main Feb 4, 2024
3 checks passed
@lolodomo lolodomo added the bug An unexpected problem or unintended behavior of an add-on label Feb 4, 2024
@lolodomo lolodomo added this to the 4.2 milestone Feb 4, 2024
@ccutrer ccutrer deleted the jinja-no-fail-on-unknown-tokens branch February 4, 2024 15:16
austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
Home Assistant doesn't enable strict mode, so we shouldn't either

Signed-off-by: Cody Cutrer <cody@cutrer.us>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of an add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GZCGQ01LM "JINJA-transformation failed" message in openHAB Logfile, power_outage_count
3 participants