New sensor domain expiry by masarliev · Pull Request #14067 · home-assistant/core

masarliev · 2018-04-24T07:19:14Z

Description:

Domain expiry sensor.

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#5242

Example entry for `configuration.yaml` (if applicable):

sensor:
  - platform: domain_expiry
    domain: google.com
    name: google

Checklist:

The code change is tested and works locally.
Local tests pass with tox. Your PR cannot be merged unless tests pass

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated in home-assistant.github.io

If the code communicates with devices, web services, or third-party tools:

New dependencies have been added to the REQUIREMENTS variable (example).
New dependencies are only imported inside functions that use them (example).
New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
New files were added to .coveragerc.

OttoWinter

Nice 👍 I'm already looking forward to this integration.

OttoWinter · 2018-04-24T09:23:34Z

+        add_devices([DomainExpiry(sensor_name, server_name)],
+                    True)
+
+    hass.bus.listen_once(EVENT_HOMEASSISTANT_START, run_setup)


There's no need to wait until Home Assistant start with this platform. Only cert_expiry needs it so that the frontend can show it's ask certificate.

Because the DNS server is running independent of Home Assistant, here we don't need to wait.

OttoWinter · 2018-04-24T09:29:54Z

+        """Fetch the domain information."""
+        import whois
+        domain = whois.whois(self.server_name)
+        expiry = domain.expiration_date - datetime.today()


How does this handle non-UTC timezones? How does whois.whois return the expiration date? Your code is probably ok, just checking.

expiration date from whois is in UTC

Danielhiversen · 2018-04-24T07:55:07Z

+
+SCAN_INTERVAL = timedelta(hours=24)
+
+TIMEOUT = 10.0


Danielhiversen · 2018-04-24T07:55:22Z

+
+def setup_platform(hass, config, add_devices, discovery_info=None):
+    """Set up domain expiry sensor."""
+    def run_setup(event):


Why are you delaying the setup?

Danielhiversen · 2018-04-24T10:20:12Z

+            expiry = domain.expiration_date - datetime.today()
+            self._state = expiry.days
+        else:
+            _LOGGER.error("Cannot get expiry date for %s", self.server_name)


set self._state = None ?

I didn't change it because expiration date will always be datetime. This check is for some domains that don't share this information and error will be triggered only for them

frenck · 2018-04-24T18:22:49Z

IMHO opinion, the host configuration variable should be renamed to domain to better match the actual thing we are monitoring here. Secondly, the host variable is used to define the host to connect to in many many components and actually, we are not connecting to it in this case.

fanaticDavid · 2018-04-24T21:24:43Z

Valid point, but the Certificate Expiry sensor (which probably served as a point of reference here) uses host instead of domain as well.

OttoWinter · 2018-04-24T21:41:46Z

@fanaticDavid True, but that doesn't mean we should repeat past mistakes. In the end, it's not going to be that big of a deal with either choice, users will have to go to the home-assistant.io component page anyway for setting up the integration.

I personally value the direct user experience through descriptive configuration keys more than consistency. (That doesn't mean consistency is not important, it is, but that it's not that high of a priority in this case)

fanaticDavid · 2018-04-24T21:53:20Z

@OttoWinter I agree, we should not repeat past mistakes. I would change host to domain for this new integration as @frenck suggested. Personally, I'm OCD enough to be okay with a breaking change for the Certificate Expiry sensor in the name of consistency, but that's not my call to make 😜

OttoWinter · 2018-04-24T22:00:49Z

I mean one thing speaking against changing it for cert_expiry is that for that sensor it is actually a host. For example, you can use an IP address with the cert_expiry platform.

The difference is that the cert_expiry platform actually does set up a connection to the specified host (+ port!) and checks the server for a valid certificate. The domain_expiry sensor on the other hand never connects to the actual target domain and just uses DNS queries, so it does not connect to a "host".

fanaticDavid · 2018-04-24T22:19:47Z

I stand corrected! Now, back on topic... 😛

frenck · 2018-04-25T10:33:20Z

Thanks for the adjustment @masarliev! 👍

fabaff · 2018-05-10T18:05:17Z

This is pretty much a duplicate of the already available whois sensor.

masarliev · 2018-05-10T18:53:53Z

Yes. It's same. As it seems I didn't know about whois sensor. Should we delete this sensor?

balloob · 2018-05-10T19:32:17Z

Yes. Opening a PR for that now.

balloob · 2018-05-10T19:34:22Z

#14381

domain expiry

ab1aa73

homeassistant added cla-signed new-platform platform: sensor.domain_expiry labels Apr 24, 2018

masarliev mentioned this pull request Apr 24, 2018

domain expiry home-assistant/home-assistant.io#5242

Merged

2 tasks

OttoWinter reviewed Apr 24, 2018

View reviewed changes

Danielhiversen reviewed Apr 24, 2018

View reviewed changes

masarliev added 2 commits April 24, 2018 13:03

domain expiry

af585df

domain expiry

8cb3bea

homeassistant added the cla-signed label Apr 24, 2018

scan interval

6b6ea03

Danielhiversen reviewed Apr 24, 2018

View reviewed changes

syssi approved these changes Apr 25, 2018

View reviewed changes

change host to domain

4b6ccff

homeassistant added the cla-signed label Apr 25, 2018

pvizeli approved these changes Apr 25, 2018

View reviewed changes

pvizeli merged commit f23f946 into home-assistant:dev Apr 25, 2018

balloob mentioned this pull request May 10, 2018

Remove domain expiry sensor #14381

Merged

balloob mentioned this pull request May 11, 2018

0.69 #14392

Merged

home-assistant locked and limited conversation to collaborators Sep 5, 2018

ghost added integration: domain_expiry and removed platform: sensor.domain_expiry labels Mar 21, 2019


		SCAN_INTERVAL = timedelta(hours=24)

		TIMEOUT = 10.0

Uh oh!

Conversation

masarliev commented Apr 24, 2018 • edited by fabaff Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Uh oh!

OttoWinter left a comment

Choose a reason for hiding this comment

Uh oh!

OttoWinter Apr 24, 2018

Choose a reason for hiding this comment

Uh oh!

OttoWinter Apr 24, 2018

Choose a reason for hiding this comment

Uh oh!

masarliev Apr 24, 2018

Choose a reason for hiding this comment

Uh oh!

Danielhiversen Apr 24, 2018

Choose a reason for hiding this comment

Uh oh!

Danielhiversen Apr 24, 2018

Choose a reason for hiding this comment

Uh oh!

Danielhiversen Apr 24, 2018

Choose a reason for hiding this comment

Uh oh!

masarliev Apr 24, 2018

Choose a reason for hiding this comment

Uh oh!

frenck commented Apr 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fanaticDavid commented Apr 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

OttoWinter commented Apr 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fanaticDavid commented Apr 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

OttoWinter commented Apr 24, 2018

Uh oh!

fanaticDavid commented Apr 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

frenck commented Apr 25, 2018

Uh oh!

fabaff commented May 10, 2018

Uh oh!

masarliev commented May 10, 2018

Uh oh!

balloob commented May 10, 2018

Uh oh!

balloob commented May 10, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

masarliev commented Apr 24, 2018 •

edited by fabaff

Loading

Example entry for `configuration.yaml` (if applicable):

frenck commented Apr 24, 2018 •

edited

Loading

fanaticDavid commented Apr 24, 2018 •

edited

Loading

OttoWinter commented Apr 24, 2018 •

edited

Loading

fanaticDavid commented Apr 24, 2018 •

edited

Loading

fanaticDavid commented Apr 24, 2018 •

edited

Loading