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

Fix recorder purge #10318

Merged
merged 3 commits into from Nov 3, 2017
Merged

Fix recorder purge #10318

merged 3 commits into from Nov 3, 2017

Conversation

pvizeli
Copy link
Member

@pvizeli pvizeli commented Nov 3, 2017

Description:

Fix wrong syntax from: #10279

@lwis
Copy link
Member

lwis commented Nov 3, 2017

I'm also seeing a weird issue on line 280;

hass_1  | 2017-11-03 16:29:56 ERROR (MainThread) [homeassistant.components.recorder.util] Error executing quer
y: Not naive datetime (tzinfo is already set)                                                                 
hass_1  | 2017-11-03 16:29:56 ERROR (MainThread) [homeassistant.core] Error doing job: Exception in callback $
function Recorder.run.<locals>.register at 0x7f570847c840>                                                    
hass_1  | Traceback (most recent call last):
hass_1  |   File "uvloop/cbhandles.pyx", line 47, in uvloop.loop.Handle._run (uvloop/loop.c:48400)
hass_1  |   File "/usr/lib/python3.6/site-packages/homeassistant/components/recorder/__init__.py", line 280, i
n register
hass_1  |     run = dt_util.UTC.localize(event.time_fired) + \
hass_1  |   File "/usr/lib/python3.6/site-packages/pytz/__init__.py", line 237, in localize
hass_1  |     raise ValueError('Not naive datetime (tzinfo is already set)')
hass_1  | ValueError: Not naive datetime (tzinfo is already set)

@PeteBa
Copy link
Contributor

PeteBa commented Nov 3, 2017

@pvizeli , sorry to have caused inconvenience and thanks for catching this. PR #10279 passed both local tests and tox that exercised these lines so keen to understand what went wrong here. I thought lines 363-364 were sufficient to provide a valid function reference for use later in the code:

363:                 async_track_point_in_time = \
364:                     self.hass.helpers.event.async_track_point_in_time

Appreciate any feedback you can spare on this.

@pvizeli
Copy link
Member Author

pvizeli commented Nov 3, 2017

@PeteBa correct
@lwis I address you comment with last commit

@lwis
Copy link
Member

lwis commented Nov 3, 2017

@pvizeli 👍

@balloob balloob merged commit a4dec0b into dev Nov 3, 2017
@balloob balloob deleted the fix-purge branch November 3, 2017 19:55
@PeteBa
Copy link
Contributor

PeteBa commented Nov 3, 2017

@lwis, for my benefit, can I ask what database you are using with recorder. Is it sqlite or something different ?

@lwis
Copy link
Member

lwis commented Nov 3, 2017

@PeteBa postgres

@PeteBa
Copy link
Contributor

PeteBa commented Nov 3, 2017

@lwis, thanks

OK, so lesson learned is some db return timezone aware timestamps whereas others are naive (that and testing on sqlite alone is not enough !) For future reference, there is a method in recorder/model.py that provides some db normalisation i.e. event.to_native().time_fired would work too.

balloob pushed a commit that referenced this pull request Nov 4, 2017
* Fix recorder purge

* Fix lint

* fix utc convert
@balloob balloob mentioned this pull request Nov 4, 2017
@home-assistant home-assistant locked and limited conversation to collaborators Mar 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants