frenck / spook

A scary 👻 powerful toolbox 🧰 for Home Assistant 🏡
https://spook.boo
MIT License
381 stars 36 forks source link

thinks a entity is not there...but it is, could it be a ghost entity?? #18

Closed Anto79-ops closed 1 year ago

Anto79-ops commented 1 year ago

Hi, awesome integration.

Not sure, but the helper thinks I'm missing an entity:

image

This sensor is created by Stephen Beechen "Home Assistant Google Drive Backup"

anyways, the entity is alive and well

image

and wanted to bring this to your attention.

Cheers

frenck commented 1 year ago

This sensor is created by Stephen Beechen "Home Assistant Google Drive Backup"

Which integration would that be?

anyways, the entity is alive and well

Interesting, what does it look like in the developer states?

../Frenck

Anto79-ops commented 1 year ago

its an addon

Home Assistant Google Drive Backup

https://github.com/sabeechen/hassio-google-drive-backup

but it might be a transient thing because now the warning message is gone and I don't remember pressing ignore....but shows up on restart

image

frenck commented 1 year ago

That is not an issue I can and will solve here. In this case, the add-on is abusing the API, which is not correct and should be reported and fixed by the addon.

References:

https://github.com/sabeechen/hassio-google-drive-backup/blob/3b5936e59769000823118edc2d4ebcc8f9e90200/hassio-google-drive-backup/backup/ha/harequests.py#L328-L329

https://github.com/sabeechen/hassio-google-drive-backup/blob/3b5936e59769000823118edc2d4ebcc8f9e90200/hassio-google-drive-backup/backup/ha/harequests.py#L265-L268

Which is basically doing a direct post request to inject a state into Home Assistant. It injects/creates it, which is not the way it should be done.

There are fundamental issues with this, and not how this is supposed to work. Some less technical background on this:

https://www.home-assistant.io/blog/2021/05/12/integrations-api/

This can be correctly solved by the author in multiple ways:

  1. Add a proper integration to Home Assistant that actually communicates with the add-on (and/or the add-on with the integration).
  2. The add-on could use an existing protocol (like MQTT, which supports discovery).
  3. Expose an API that a user can query using, e.g., the rest integration.

All of which, the first solution is generally considered the one with the best experience.

The current implementation in the add-on makes the entity (and its existence) unpredictable, fully unaware, and unregistered. This is not just an issue for this integration, but for every dashboard, action/condition/trigger, and the internal registries (as now the entity ID isn't guaranteed and the add-on could be causing collisions and updating something completely different).

I would strongly recommend reporting this issue upstream.

../Frenck

Anto79-ops commented 1 year ago

ok, I will post on the Home Assistant Google Drive Backup page.

Thanks for this information

EDIT: reported here https://github.com/sabeechen/hassio-google-drive-backup/issues/805

frenck commented 1 year ago

Cool, let's see if that improves, would be nice 👍

That API capability/endpoint he is using is soo darn old and shouldn't be used for this. Will check @ Core team as well, if it might be time to mark it officially deprecated and let it throw warnings or something.

It really isn't a good idea to use it for this.

../Frenck

sabeechen commented 1 year ago

For any users of the Home Assistant Google Drive Backup addon that wish to avoid this issue, I have posted a workaround here.

A companion integration would be best, its on my radar for the future, but I haven't had the time to support another codebase and implement an upgrade path for my users. The degraded sensor experience certainly isn't ideal, but it was a quick way to get the ball rolling 3 years ago.

@frenck I appreciate your looking into the details of my usage but I take some issue with calling this abuse. Being an authority on Home Assistant your opinion carries weight with the community and is somewhat damaging to my reputation. Maybe you can help me understand why you say that, it looks to me like I'm using the /api/states/ endpoint exactly for its publicly documented purpose:

Updates or creates a state. You can create any state that you want, it does not have to be backed by an entity in Home Assistant.

It appears to me that this was an appropriate way to do things in the past, internally things have changed, and it may no longer be supported in the future, but abuse? Same team, man.

frenck commented 1 year ago

For any users of the Home Assistant Google Drive Backup addon that wish to avoid this issue, I have posted a workaround https://github.com/sabeechen/hassio-google-drive-backup/issues/805#issuecomment-1450982162.

👍 Awesome!

I appreciate your looking into the details

No problem at all.

The degraded sensor experience certainly isn't ideal, but it was a quick way to get the ball rolling 3 years ago.

It appears to me that this was an appropriate way to do things in the past

Sorry to say, three years ago that wasn't a correct solution either.

Linked it above too, but here is a blog about why HA doesn't want to use an API for things like this: https://www.home-assistant.io/blog/2021/05/12/integrations-api/.

but I take some issue with calling this abuse.

Sorry to hear that. Your response seems to take this personally, which is sad to hear. It is about code and its functionality, and just like in any code review, you should never take that personally. It doesn't say anything about you, your skill, or your capabilities at all, it says the code is doing something that isn't right.

If "abuse" as a term itself bothers you, feel free to replace it with "inappropriate", "incorrect", "misuse", "wrong", "incorrect", "not entirely as it should", or something similar you like better, if that makes you feel better; however, the conclusion and results remain the same.

Same team, man.

Yup, hence I dive in. I've enjoyed the use of your add-on in the past 👍 (but de-Googliefied a bit myself).

I do not mean to offend you. Nothing but good intentions.

../Frenck

sabeechen commented 1 year ago

Lets avoid making assumptions about whats going on in eachother's heads.

I think we are 100% in agreement that the best thing to do here is make a companion integration that creates proper entities in Home Assistant, apologies if what I said made you think I think otherwise. Its a deficiency that should be resolved.

What I'm most concerned with is that someone else reading this without detailed knowledge about how Home Assistant works would interpret what you wrote:

In this case, the add-on is abusing the API

as meaning that the add-on is doing something nefarious. In this case the specific language you're using is relevant, because abuse implies there is malicious intent. It isn't an accurate synonym for incorrect/misuse/etc. Abuse is what bad actors do to exploit a system. Our users read threads like these to make informed decisions about what they should or shouldn't use. If you are currently using abuse as a synonym for misuse/incorrect/etc, I ask that you consider what you classify as abuse in the future.

frenck commented 1 year ago

Lets avoid making assumptions about whats going on in eachother's heads.

Lol are you ok?

Whatever.