whois domain lookup sensor #10000
whois domain lookup sensor #10000
Conversation
| def unit_of_measurement(self): | ||
| """The unit of measurement to present the value in.""" | ||
| if self._expired: | ||
| return None |
| self.hass = hass | ||
| self._hostname = hostname | ||
| self.whois = get_whois | ||
| self._state = STATE_UNKNOWN |
There was a problem hiding this comment.
Set it to None and let Home Assistant handle the rest.
| """Get the more info attributes.""" | ||
| if self._data: | ||
| updated_formatted = self._updated_date.strftime( | ||
| '%Y-%m-%d %H:%M:%S') |
| if self._expired: | ||
| return None | ||
|
|
||
| return 'day{}'.format('' if self._state == 1 else 's') |
There was a problem hiding this comment.
The unit of measurement will change if only one day is left. This will make it hard for people who are exporting the data as the sensor will go for days to day and then to .
| # every 24 hours shouldn't count as "high volume" | ||
|
|
||
| PLATFORM_SCHEMA = PLATFORM_SCHEMA.extend({ | ||
| vol.Required(CONF_HOST): vol.All(cv.ensure_list, [cv.string]), |
There was a problem hiding this comment.
Please allow the user to choose a name for the sensor. Hint: CONF_NAME.
There was a problem hiding this comment.
Which style should I go for, Instantiate this sensor with only a single host, such as:
sensor:
- platform: whois
hostname: example.net
name: primary
- platform: whois
hostname: example.com
name: secondaryor keep the list of hostnames and add the per host name config, such as:
sensor:
- platform: whois
hostnames:
- url: example.net
name: primary
- url: example.com
name: secondaryI can see merits in both but was curious if there was a process / design used to decide which one to go for.
Thanks for your feedback on this all, much appreciated.
There was a problem hiding this comment.
I personally prefer the first style (easier for copy-&-paste) but this is pretty much up to the person who is writing the platform.
| """Initialize the sensor.""" | ||
| from pythonwhois import get_whois | ||
|
|
||
| self.hass = hass |
|
|
||
| def update(self): | ||
| """Get the current WHOIS data for hostname.""" | ||
| response = self.whois(self._hostname, normalized=True) |
There was a problem hiding this comment.
I guess that there will be an exception if the domain doesn't exist. You need to catch it.
| devices = [] | ||
|
|
||
| for hostname in hostnames: | ||
| devices.append(WhoisSensor(hass, hostname)) |
There was a problem hiding this comment.
You should make sure that the domain exists before setting up the platform otherwise the users will end up if a non-working sensor.
- Now more resiliant to invalid hostnames - Removed various assumed STATE_UNKOWN setting - Upfront check for valid hostname preventing the sensor starting with dud - Resolved unit of measurement Day, Days, None issue - Datetime formatting now done to iso 8601 standard - Removed all expired usage, not really that useful - Unused hass assignment
- Resolved the dilemma with hosts / single host per sensor. Now running single domain per sensor. - Renamed host(s) to domain
.coveragerc
Outdated
| @@ -223,6 +223,8 @@ omit = | |||
| homeassistant/components/wemo.py | |||
| homeassistant/components/*/wemo.py | |||
|
|
|||
| homeassistant/components/sensor/whois.py | |||
There was a problem hiding this comment.
Please move it down to the platforms.
| name = config.get(CONF_NAME) | ||
|
|
||
| try: | ||
| if 'expiration_date' in get_whois(domain, normalized=True): |
There was a problem hiding this comment.
Especially for European domains is the expiration_date not always present. So the lookup is working but we will not get the data.
There was a problem hiding this comment.
Not much we can do here. Perhaps re-phrasing the warning.
| self._name_servers = self._data['nameservers'] | ||
|
|
||
| self._expiration_date = self._data['expiration_date'][0] | ||
| self._updated_date = self._data['updated_date'][0] |
There was a problem hiding this comment.
With home-assistant.io I get:
File "/home/fab/Documents/repos/home-assistant/homeassistant/components/sensor/whois.py", line 131, in update
self._updated_date = self._data['updated_date'][0]
KeyError: 'updated_date'There was a problem hiding this comment.
Seems my assumption of its existence was incorrect, I've made it conditional now. Resolved the other comments too.
Description:
This is at the "test the water" PR stage in that I have developed the basics of the sensor but wish to get feedback on whether this is a suitable component for HA.
I've used the
pythonwhoislibrary that comes with awtfpllicence, and have pinned theSCAN_INTERVALat 24 hours as we don't want to break the WHOIS TOS withhigh volumerequests. I feel that one request per domain every 24 hours should be ok.Is this a desirable feature and should I fully flesh it out with tests / doc PR?
Current State
Creates a sensor for each host with the following information:
Any feedback much welcomed :)
Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#3744
Example entry for
configuration.yaml(if applicable):Checklist:
If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
toxrun successfully. Your PR cannot be merged unless tests passREQUIREMENTSvariable (example).requirements_all.txtby runningscript/gen_requirements_all.py..coveragerc.