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

Handle non-string values in JSON template renderer #20233

Merged
merged 1 commit into from
Jan 21, 2019

Conversation

pnbruckner
Copy link
Contributor

Description:

Using the SQL Sensor to extract a datetime value for a state (such as last_changed), and using value_template to convert that datetime to another format, was resulting in a TypeError being thrown by json.loads, indirectly called by async_render_with_possible_json_value in homeassistant.helpers.template. This error should be ignored so that the template can be rendered without value_json (just like what happens with a malformed JSON string.)

This change fixes that problem by catching TypeError in addition to ValueError.

Example error from log:

sql: Error on device update!
Traceback (most recent call last):
  File "/srv/homeassistant/lib/python3.5/site-packages/homeassistant/helpers/entity_platform.py", line 248, in _async_add_entity
    await entity.async_device_update(warning=False)
  File "/srv/homeassistant/lib/python3.5/site-packages/homeassistant/helpers/entity.py", line 349, in async_device_update
    await self.hass.async_add_executor_job(self.update)
  File "/usr/lib/python3.5/asyncio/futures.py", line 380, in __iter__
    yield self  # This tells Task to wait for completion.
  File "/usr/lib/python3.5/asyncio/tasks.py", line 304, in _wakeup
    future.result()
  File "/usr/lib/python3.5/asyncio/futures.py", line 293, in result
    raise self._exception
  File "/usr/lib/python3.5/concurrent/futures/thread.py", line 55, in run
    result = self.fn(*self.args, **self.kwargs)
  File "/srv/homeassistant/lib/python3.5/site-packages/homeassistant/components/sensor/sql.py", line 160, in update
    data, None)
  File "/srv/homeassistant/lib/python3.5/site-packages/homeassistant/helpers/template.py", line 166, in async_render_with_possible_json_value
    variables['value_json'] = json.loads(value)
  File "/usr/lib/python3.5/json/__init__.py", line 312, in loads
    s.__class__.__name__))
TypeError: the JSON object must be str, not 'datetime'

Related issue (if applicable): none

Pull request in home-assistant.io with documentation (if applicable): none

Example entry for configuration.yaml (if applicable):

sensor:
  - platform: sql
    db_url: !secret db_url
    queries:
      - name: Get last_changed in local time
        query: "SELECT last_changed FROM states WHERE entity_id = 'domain.object_id' ORDER BY state_id DESC LIMIT 1;"
        column: last_changed
        value_template: "{{ strptime(value~'+0000', '%Y-%m-%d %H:%M:%S%z').astimezone(now().tzinfo) }}"

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

Handle the case of async_render_with_possible_json_value's value argument
being something other than a string. This can happen, e.g., when using the
SQL sensor to extract a datetime column such as last_changed and also using
its value_template to convert that datetime to another format. This was
causing a TypeError from json.loads, but async_render_with_possible_json_value
was only catching ValueError's.
@pnbruckner pnbruckner requested a review from a team as a code owner January 18, 2019 22:08
@homeassistant homeassistant added cla-signed core small-pr PRs with less than 30 lines. labels Jan 18, 2019
@ghost ghost added the in progress label Jan 18, 2019
@pnbruckner pnbruckner changed the title Handle non-string values in JSON renderer Handle non-string values in JSON template renderer Jan 19, 2019
@balloob balloob added this to the 0.86.0 milestone Jan 21, 2019
Copy link
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

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

Nice catch!

@balloob balloob merged commit 935e5c6 into home-assistant:dev Jan 21, 2019
@ghost ghost removed the in progress label Jan 21, 2019
balloob pushed a commit that referenced this pull request Jan 21, 2019
Handle the case of async_render_with_possible_json_value's value argument
being something other than a string. This can happen, e.g., when using the
SQL sensor to extract a datetime column such as last_changed and also using
its value_template to convert that datetime to another format. This was
causing a TypeError from json.loads, but async_render_with_possible_json_value
was only catching ValueError's.
@pnbruckner pnbruckner deleted the template branch January 21, 2019 15:02
@balloob balloob mentioned this pull request Jan 23, 2019
alandtse pushed a commit to alandtse/home-assistant that referenced this pull request Feb 12, 2019
Handle the case of async_render_with_possible_json_value's value argument
being something other than a string. This can happen, e.g., when using the
SQL sensor to extract a datetime column such as last_changed and also using
its value_template to convert that datetime to another format. This was
causing a TypeError from json.loads, but async_render_with_possible_json_value
was only catching ValueError's.
kellerza pushed a commit to kellerza/home-assistant that referenced this pull request Feb 24, 2019
Handle the case of async_render_with_possible_json_value's value argument
being something other than a string. This can happen, e.g., when using the
SQL sensor to extract a datetime column such as last_changed and also using
its value_template to convert that datetime to another format. This was
causing a TypeError from json.loads, but async_render_with_possible_json_value
was only catching ValueError's.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants