search

home-assistant-ecosystem / home-assistant-cli

:computer: Command-line tool for Home Assistant
Other
446 stars 69 forks source link

fix `hass-cli state history --since .... ` by correcting the URL_API_HISTORY_PERIOD constant #418

Open varenc opened 3 months ago

varenc commented 3 months ago

This fixes a key bug with hass-cli state history --since '.....'. The bug is that it ignores the value of --since completely.

The trouble is that .format() is being used here on the URL_API_HISTORY_PERIOD constant, but because the constant contains no {} nothing actually changes. This means that the start_time being passed in is just completely ignored. This PR just fixes that to work as intended.

Perviously it would always make a request to /api/history/period?... instead of /api/history/period/<start_time>?....

I'm guessing it used to work but this was lost in a refactor.

varenc commented 3 months ago

Sure enough, it looks like when --since and --end were first added 6 years ago that URL_API_HISTORY_PERIOD constant is what I would expect: https://github.com/home-assistant-ecosystem/home-assistant-cli/blob/b93ab247c0586d7117af96e231604faa4106a85d/homeassistant_cli/hassconst.py#L410

So seems like this was broken in a refactor. The bug is subtle and only matter if you care about getting more history than the default history length.

varenc commented 3 months ago

I think a better approach would be to remove {} format values from all constants completely which would make bugs like this much more obvious in the future. But looks like its used in a few other places as well so seems like that should be part of a larger refactor.