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.

Sign up for GitHub

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

Jump to bottom

[mapdb] Fix missing service pid #16567

Merged
merged 2 commits into from Mar 27, 2024
Merged

[mapdb] Fix missing service pid #16567

merged 2 commits into from Mar 27, 2024

Conversation

J-N-K
Copy link
Member

@J-N-K J-N-K commented Mar 24, 2024

No description provided.

@J-N-K
[mapdb] Fix missing service pid
c07f655 
Signed-off-by: Jan N. Klug <github@klug.nrw>
@J-N-K J-N-K added the bug An unexpected problem or unintended behavior of an add-on label Mar 24, 2024
@J-N-K J-N-K requested a review from a team as a code owner March 24, 2024 11:34
@J-N-K
fix spotless
b8c81b7 
Signed-off-by: Jan N. Klug <github@klug.nrw>
@jlaur
Copy link
Contributor

jlaur commented Mar 26, 2024

@J-N-K - I found #9132 as reference which added property = Constants.SERVICE_PID + "=org.openhab.jdbc" for @Component in JdbcPersistenceService. Out of curiosity, and since it has been missing for years, what does it do? And what kind of issues could be expected when this is missing?

@J-N-K
Copy link
Member Author

J-N-K commented Mar 27, 2024

Besides being incosistent, it's not creating an issue ATM (at least none that I'm aware of). I'm working on an improvement to the persistence configuration which relies on a properly set service id, that's why I discovered it.

@jlaur
Copy link
Contributor

jlaur commented Mar 27, 2024

Besides being incosistent, it's not creating an issue ATM (at least none that I'm aware of). I'm working on an improvement to the persistence configuration which relies on a properly set service id, that's why I discovered it.

The inconsistency could also have been resolved by removing the property elsewhere if it was not actually used for anything. Thanks for the explanation, and now it seems it will become used.

jlaur
jlaur approved these changes Mar 27, 2024
View reviewed changes
Copy link
Contributor

@jlaur jlaur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jlaur jlaur merged commit a4ee09a into openhab:main Mar 27, 2024
3 checks passed
@jlaur jlaur added this to the 4.2 milestone Mar 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jlaur jlaur jlaur approved these changes

Assignees
No one assigned
Labels
bug An unexpected problem or unintended behavior of an add-on
Projects
None yet
Milestone
4.2
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@J-N-K @jlaur