这是indexloc提供的服务,不要输入任何密码
Skip to content
This repository was archived by the owner on May 17, 2021. It is now read-only.

Conversation

@lewie
Copy link
Contributor

@lewie lewie commented Feb 15, 2016

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.

@watou
Copy link
Contributor

watou commented Feb 15, 2016

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.

@lewie
Copy link
Contributor Author

lewie commented Feb 15, 2016

@watou,
this info-changes based on questions from the Forum to the installation and connectivity of the JDBC Persistence. These messages inform that the database could be contacted and appear only once at the start of the framework.
If you insist, I'll put them back willingly to debug. ;-)

@lewie
Copy link
Contributor Author

lewie commented Feb 15, 2016

@watou,

 -            logger.warn("JDBC::store: ignore Item because it is UnDefType");
+            logger.info("JDBC::store: ignore Item '{}' because it is UnDefType", item.getName());

Hmm, this is not only at startup. If a Item is UnDefType it can not be persisted, so this is a unusable state.
I want to know about that, without to switch to debug.

@watou
Copy link
Contributor

watou commented Feb 15, 2016

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.

@lewie
Copy link
Contributor Author

lewie commented Feb 15, 2016

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}">
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Member

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>

Copy link
Contributor Author

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.

Copy link
Member

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.

Copy link
Member

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}.

Copy link
Member

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

Copy link
Contributor Author

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.

Copy link
Member

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.

Copy link
Contributor Author

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

@lewie lewie force-pushed the add_jdbc_to_paperui_and_features branch from 7aee28d to 9e6de8c Compare February 16, 2016 06:24
@lewie
Copy link
Contributor Author

lewie commented Feb 16, 2016

@watou, now I have a single info message at startup and warn messages on problems for "UnDefType".
@kaikreuzer, review round 1 implemented.

@kaikreuzer
Copy link
Member

Perfect, thanks!

kaikreuzer added a commit that referenced this pull request Feb 16, 2016
Add jdbc Persistence to Paperui and features
@kaikreuzer kaikreuzer merged commit b30645f into openhab:master Feb 16, 2016
lewie added a commit to lewie/openhab that referenced this pull request Mar 14, 2016
lewie added a commit to lewie/openhab that referenced this pull request Mar 14, 2016
@lewie
Copy link
Contributor Author

lewie commented Mar 14, 2016

@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?

@lewie lewie deleted the add_jdbc_to_paperui_and_features branch March 14, 2016 18:22
@kaikreuzer
Copy link
Member

@lewie I saw many exceptions in my log when the service tried to start without any proper configuration set. I have tried to fix it in #4266 - with this, the service is now correctly activated and I see the configuration screen as expected:

bildschirmfoto 2016-04-08 um 10 48 32

@lewie
Copy link
Contributor Author

lewie commented Apr 8, 2016

Thank You very much @kaikreuzer!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants