-
Notifications
You must be signed in to change notification settings - Fork 601
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
Time handling #2901
Time handling #2901
Conversation
…ervisor into time-services
def set_time(self, utc: datetime): | ||
"""Set time & date on host as UTC. | ||
|
||
Return a coroutine. | ||
""" | ||
return self.dbus.SetTime(int(utc.timestamp() * 1000000), False, False) | ||
|
||
@dbus_connected | ||
def set_ntp(self, use_ntp: bool): | ||
"""Turn NTP on or off. | ||
|
||
Return a coroutine. | ||
""" | ||
return self.dbus.SetNTP(use_ntp) |
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.
I know that we used to do this in the past, but it has caused me quite some headaches with debugging. I think it's more readable if we use async methods and await:
def set_time(self, utc: datetime): | |
"""Set time & date on host as UTC. | |
Return a coroutine. | |
""" | |
return self.dbus.SetTime(int(utc.timestamp() * 1000000), False, False) | |
@dbus_connected | |
def set_ntp(self, use_ntp: bool): | |
"""Turn NTP on or off. | |
Return a coroutine. | |
""" | |
return self.dbus.SetNTP(use_ntp) | |
async def set_time(self, utc: datetime): | |
"""Set time & date on host as UTC.""" | |
return await self.dbus.SetTime(int(utc.timestamp() * 1000000), False, False) | |
@dbus_connected | |
async def set_ntp(self, use_ntp: bool): | |
"""Turn NTP on or off.""" | |
return await self.dbus.SetNTP(use_ntp) |
Although I realize that @dbus_connected
would need to have support for async methods.
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.
I think using typing would solve the problem without making it slow and eat more filehandle which is a limited resource on the kernel. -> Awaitable[]
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.
We don't set Time and NTP enough that speed or file handles are a concern.
@@ -241,7 +241,7 @@ async def check_connectivity(self): | |||
timeout = aiohttp.ClientTimeout(total=10) | |||
try: | |||
await self.sys_websession.head( | |||
"http://version.home-assistant.io/online.txt", timeout=timeout | |||
"https://version.home-assistant.io/online.txt", timeout=timeout |
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.
Could this cause errors when we check connectivity and the time is out of sync ?
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.
Correct, but we use SSL everywhere, the whole system is not functional if the time is out of sync too much. This is our main problem for 3 years and makes that system comes never function or hang in an endless loop because we can't install or setup containers etc. This PR going to fix that close as possible for now.
from ..exceptions import WhoamiConnectivityError, WhoamiError, WhoamiSSLError | ||
from .dt import utc_from_timestamp | ||
|
||
_LOGGER: logging.Logger = logging.getLogger(__name__) |
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.
I think that typing like this will be infered by MyPy.
_LOGGER: logging.Logger = logging.getLogger(__name__) | |
_LOGGER = logging.getLogger(__name__) |
Ah my bad, this is the default redirect. It does work.
|
Right: |
|
||
UTC = pytz.utc | ||
|
||
GEOIP_URL = "http://ip-api.com/json/" |
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.
🎉
Co-authored-by: Joakim Sørensen <joasoe@gmail.com>
Proposed change
Resolve all issues at the wrong time. It gets all information from the host and can (if needed) adjust the time also over our new whoiam service. It also allows to use of the timezone information from the host on supervisors.
We using the systemd timedate1 backend.
Type of change
Additional information
Checklist
black --fast supervisor tests
)If API endpoints of add-on configuration are added/changed: