Skip to content
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

[persistence-addons] Implement QueryablePersistenceService where appropriate #12663

Open
TBail opened this issue Dec 15, 2021 · 10 comments
Open
Labels
enhancement An enhancement or new feature for an existing add-on persistence

Comments

@TBail
Copy link

TBail commented Dec 15, 2021

While working on some persistence rules i tried to use the API to save some data to a persistence service. Unfortunately this idi not work.

To find out what is wrong with my code i tried the same with the api explorer in OH3 latest nightly. I used the

PUT /persistence/items/{itemname}

with appropriate parameters, but unfortunaltely i got an error with both services installed. i used (influxdb and mapdb). The error is

{
  "error": {
    "message": "Persistence service not modifiable: mapdb",
    "http-code": 400
  }
}

To ensure the the parameters like service id and time etc are working in general i tried ti retrieve persistence data. This worked very well.

Based on this i assume that writing persistence data with thw rest api does not work.

@cweitkamp
Copy link
Contributor

cweitkamp commented Jan 1, 2022

Thanks for reporting this missing feature. To store values in persistence via REST interface the services have to implement the ModifiablePersistenceService interface - which extends the QueryablePersistenceService interface. After a quick look into Add-Ons repository none of the available persistence services implements the mentioned feature.

I will try to have a look later into mapdb and jdbc how it can be archived.

@cweitkamp cweitkamp changed the title [Persistence] API to save data not working [persistence] API to save data not working Jan 1, 2022
@cweitkamp cweitkamp changed the title [persistence] API to save data not working [persistence] REST API to save data not working / missing feature in persistence services Jan 1, 2022
@spacemanspiff2007
Copy link

Thank you @cweitkamp for stepping in! That's really nice of you.

@cweitkamp
Copy link
Contributor

Does it really make sense to implement this interface for MapDB as it only can store the last value? Please continue discussion in #11923.

wborn referenced this issue in wborn/openhab-core Jan 2, 2022
…erface

If PRs are created to implement this interface, it would be better if we can prevent the propagation of old APIs like Date.

Related to:

* openhab/openhab-addons#11922
* openhab/openhab-addons#11923
* https://github.com/openhab/openhab-core/issues/2618#issuecomment-1003544762

Signed-off-by: Wouter Born <github@maindrain.net>
cweitkamp referenced this issue in openhab/openhab-core Jan 2, 2022
…nceService interface (#2660)

If PRs are created to implement this interface, it would be better if we can prevent the propagation of old APIs like Date.

Related to:

* openhab/openhab-addons#11922
* openhab/openhab-addons#11923
* https://github.com/openhab/openhab-core/issues/2618#issuecomment-1003544762

Signed-off-by: Wouter Born <github@maindrain.net>
@lujop
Copy link
Contributor

lujop commented Jan 9, 2022

@cweitkamp It's possible to implement only the store part without removing one and throwing a not-supported exception?

InfuxDB 2.0 it's not very supportive in deletes, and implementing full FilterCriteria won't be possible and require some work.
Because delete predicates are totally different than query predicates and the API doesn't support to construct them, and there are operators like inequality that are not supported (see https://docs.influxdata.com/influxdb/cloud/reference/syntax/delete-predicate)
However the store part it's trivial and ready to submit a PR.

@cweitkamp
Copy link
Contributor

Yes, I think so. Just throw an UnsupportedOperationException in the remove method. That's it.

@lujop
Copy link
Contributor

lujop commented Jan 9, 2022

I've added #12013 to track InfluxDB implementation

@cweitkamp
Copy link
Contributor

Thanks. I have updated my list in #12663.

@TBail
Copy link
Author

TBail commented Mar 18, 2022

I have found out the the persistence service is taking the timestamp from the database system and not from the date/time the item is changed/updated. In my case i have differences between the timestamps sometimes of mor than half an hour.

Maybe it is an approach to set the timestamp in general from openhab and not from the database system.

@openhab-bot
Copy link
Collaborator

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/persistence-irritation/134307/7

@J-N-K
Copy link
Member

J-N-K commented Apr 25, 2022

@TBail Using different time sources sounds like a bad idea. However, it is up to the persistence service implementation to determine the correct way to store values. I had a look at JDBC and it would be easy to use the ZonedDateTime.now() as timestamp when no timestamp is given (instead of passing only the value to the DB server).

@J-N-K J-N-K changed the title [persistence] REST API to save data not working / missing feature in persistence services [persistence-addons] Implement QueryablePersistenceService where appropriate Apr 29, 2022
@J-N-K J-N-K transferred this issue from openhab/openhab-core Apr 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on persistence
Projects
None yet
Development

No branches or pull requests

6 participants