-
-
Notifications
You must be signed in to change notification settings - Fork 422
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
Refactored persistence api to org.openhab and removed compatibility layer for persistence services #680
Conversation
…ayer for persistence services Signed-off-by: Kai Kreuzer <kai@openhab.org>
That's wrong. I know about a proprietary product that is using the persistence API 😉 |
Hm, well. Do I have to read that as a veto against the change or is it something the proprietary solution could cope with? |
I need to check if the specific solutions could be migrated. It will take some time. |
Can all of persistence please live in its own bundle instead of parts of it in the model bundle and parts of it in the core bundle? In my fork I created a single persistence bundle and that works wonderful (there is one class in model that is actually not model specific). If you keep the eclipse package names in that first refactoring, the resulting bundles (core+persistence) are a drop-in solution for Markus. In a second step you would start the renaming. Markus will keep the bundle with the eclipse package names for his solution and openhab core starts to use the new naming. How does that sound? I would create a PR with my changes, but my core bundle was reorganised even more (scheduler parts into scheduler bundle, thing parts into thing / thing-api bundle and so on. core is a conglomerate of non related functionality!) |
If you use the persistence API programmatically it should be already possible without the model ones (IIRC). |
I think it was only this utility class https://github.com/openhab/openhab-core/blob/master/bundles/org.openhab.core.model.persistence/src/org/eclipse/smarthome/model/persistence/extensions/PersistenceExtensions.java that I moved over and adapted. I would need to look that up, but as far as i remember it had to do with the rest interface. The persistence subsystem is not only about a persistence provider and its configuration but also about getting historic states for an item (without the xtend model stuff) |
If it is an issue for you, I am also ok to postpone this move and keep the persistence implementations in openhab1 for the 2.5 release. We could then change the namespace together with all the rest of the code for the 3.x development (which we should imho start soon). |
@maggu2810 Wdyt wrt to my last comment? |
I checked the sources. import org.eclipse.smarthome.core.persistence.FilterCriteria;
import org.eclipse.smarthome.core.persistence.FilterCriteria.Ordering;
import org.eclipse.smarthome.core.persistence.HistoricItem;
import org.eclipse.smarthome.core.persistence.ModifiablePersistenceService;
import org.eclipse.smarthome.core.persistence.PersistenceItemInfo;
import org.eclipse.smarthome.core.persistence.PersistenceService; So, as long as the package name has been changed only and no function is lost, this can be changed easily and there is no reason to postpone the package name changes. |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/how-to-develop-a-new-persistence-addon/73105/7 |
@kaikreuzer This is still marked as draft. Do we wait for additional comments? |
@maggu2810 I was mainly waiting for your feedback (thanks for it). Meanwhile some changes have been done to dynamodb in the 1.x repo, so I will have to sync this here. Will try to do this within the next days and remove the "draft status" then. |
@kaikreuzer I am happy to help out with the dynamodb during the next 1-3 weeks if that's ok? Would like to see if the various dependencies can be declared as maven dependencies, and not bundled in lib folder. Should we aim to do first #5275 (basically migration of repo, and conversion to bnd if I understand correctly)? I understand this PR would then follow, with little or no impact to persistence bundle code. |
Hey @ssalonen, Sure, it would be great if you could help on that!
We will need to prepare all three PRs at the same time, because the distro will break if any of the three is merged without the other ones. |
All, I have tonight continued to work on https://github.com/openhab/openhab2-addons/pull/5275. Overall, it seems to me that the services would need quite some testing after the migration to make sure that nothing broke. It might be better to wait until after the 2.5 release and target that change for the 3.0 development instead. Wdyt? Btw, I'd hope that the bnd migration is through soon and that we would be able to do a 2.5 release towards end of May. Could that be realistic? |
On the migration: We have only three bindings left, enocean makes good progress since today, hueemulation looks nearly finished, the only remaining is homekit. If the others are done and no progress there, I would migrate, even if I would prefer to get the refactoring merged before. That should be finished in two weeks latest. |
I'm hoping that hueemulation is done in about 3 days max. I have 90% test coverage and 100% Nullpointer annotations so generally a high coding standard and two active community testers with another two in the queue. I tried to resolve the homekit merge conflict today but gave up. It's just one file but I don't want to break anything and didn't really get the logic behind the changes. |
@J-N-K Don't forget about Z-Wave and Zigbee bindings... |
@kaikreuzer is anyone working on that? |
Not me... |
Do we still wait for the migration to complete or do we wait for a 2.5 release before we continue? |
As mentioned above, I would want to wait until after the 2.5 release. |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/poll-which-oh1-x-addons-do-you-use/79121/1 |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/poll-which-oh1-x-addons-do-you-use/79121/1 |
5 similar comments
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/poll-which-oh1-x-addons-do-you-use/79121/1 |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/poll-which-oh1-x-addons-do-you-use/79121/1 |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/poll-which-oh1-x-addons-do-you-use/79121/1 |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/poll-which-oh1-x-addons-do-you-use/79121/1 |
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/poll-which-oh1-x-addons-do-you-use/79121/1 |
The new website's Markdown renderer (like GitHub's) doesn't support tables without headers. Signed-off-by: Yannick Schaus <github@schaus.net>
The persistence API is not yet used anywhere apart from the 1.x compatibility layer.
As discussed in the past, it makes a lot of sense to switch this to the openhab namespace right away, which makes it almost identical to the old 1.x persistence API. As a result, we do not require the compatibility layer for persistence services anymore and those can be moved from openhab1-addons to openhab2-addons in consequence.
This PR must only be merged, once the PRs in openhab1-addons and openhab2-addons are ready to be merged as well.
Signed-off-by: Kai Kreuzer kai@openhab.org