-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add jdbc Persistence to Paperui and features #4049
Add jdbc Persistence to Paperui and features #4049
Conversation
|
Very nice! I see you changed a number of warns to infos; any chance a number of them could be lowered even further to debug or trace? Generally, I don't think users will want to see those log messages every time. |
|
@watou, |
Hmm, this is not only at startup. If a Item is UnDefType it can not be persisted, so this is a unusable state. |
|
I would never insist! :-) But my preference is to only have a single info message at startup, warn messages on problems, and as many debug messages as would help troubleshoot. |
|
Ok, a compromise, I will gather the startup-states in a variable and send them as a single info on framework start. "UnDefType" tells about a problem, so will set back to warn. |
| <bundle start-level="80">mvn:org.openhab.persistence/org.openhab.persistence.jdbc/${project.version}</bundle> | ||
| </feature> | ||
|
|
||
| <feature name="jdbc-driver-derby" description="JDBC Driver derby-10.12.1.1" version="${project.version}"> |
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.
Why did you define features for all the drivers as well? Wouldn't it be good enough to add the bundle to the persistence service features above?
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.
JPA Persistence, will use the drivers later too. So I externalized them (the user doesn`t see them, or is it?). If we load them all by default, this would cause over 15 MB Traffic.
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.
JPA Persistence, will use the drivers later too
Ok, but as long as the feature only includes a single bundle, you can also reference the single bundle in the JPA Persistence and do not necessarily require a feature. A feature mainly makes sense, if you need to package multiple bundles and/or additional resources or configurations with it.
the user doesn`t see them
Correct, the will only be visible in the shell when listing available features. But that's ok.
If we load them all by default, this would cause over 15 MB Traffic.
This was anyhow not my suggestion. I simply meant you could also do a
<feature name="openhab-persistence-jdbc-derby" description="JDBC Persistence Apache Derby" version="${project.version}">
<configfile finalname="${openhab.conf}/services/jdbc.cfg" override="false">mvn:${project.groupId}/openhab-addons-external/${project.version}/cfg/jdbc</configfile>
<feature prerequisite="false" dependency="false">openhab-runtime-base</feature>
<feature prerequisite="false" dependency="false">openhab-runtime-compat1x</feature>
<bundle start-level="80">mvn:org.apache.derby/derbyclient/10.12.1.1</bundle>
<bundle start-level="80">mvn:org.openhab.persistence/org.openhab.persistence.jdbc/${project.version}</bundle>
</feature>
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 understand, will change it.
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.
If you plan to reuse the bundles, it might also be a good idea to put the version of the drivers as a property into the pom.xml and only refer to it here - then you can change it later at a single place and it will be picked up by JDBC and JPA persistence likewise.
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.
It should probably go best in here (all of them): https://github.com/openhab/openhab/blob/master/features/openhab-addons/pom.xml
And I would call the property simply ${derby.version}.
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.
Alternatively, you could put it in the parent pom, this is how it looks for the openhab2-addons: https://github.com/openhab/openhab2-addons/blob/master/pom.xml#L50
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.
OK, then all bundles in openHAB can use the same version of the driver later.
This has advantages for compatibility and stability?!
Can this variable from parent pom used too in local https://github.com/lewie/openhab/blob/add_jdbc_to_paperui_and_features/bundles/persistence/org.openhab.persistence.jdbc/pom.xml as ${derby.version}? If yes, I can delete it (them all) there.
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.
Yes, exactly! I wasn't aware that you need it there anyhow already - then the root pom is indeed the best place as it is shared between the bundles and the features.
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.
Thank You!
Anything went wrong with my comment wrong document, don' know...
That I can go on, please can you give me a short state about that too:
lewie@7aee28d#commitcomment-16098127
7aee28d to
9e6de8c
Compare
|
@watou, now I have a single info message at startup and warn messages on problems for "UnDefType". |
|
Perfect, thanks! |
Add jdbc Persistence to Paperui and features
enhancement for openhab#4049
|
@kaikreuzer, it was not as perfect as we thought... ;-) JDBC Persistence couldn't start without Database mandatory url, user and password. This works with this new PR. But after starting, I do still not get my superduper form, to config JDBC from ESH-INF. ;-)) What is missing? |
|
Thank You very much @kaikreuzer! |
Based on this hints added JDBC to OH2 features and PaperUI service configuration.
The JDBC Persistence can be fully installed and configured by the PaperUI now.