Skip to content

[hono] Add support for JDBC device registry#164

Merged
sophokles73 merged 1 commit into
eclipse-packages:masterfrom
lhotari:lh-hono-device-registry-jdbc
Dec 17, 2020
Merged

[hono] Add support for JDBC device registry#164
sophokles73 merged 1 commit into
eclipse-packages:masterfrom
lhotari:lh-hono-device-registry-jdbc

Conversation

@lhotari

@lhotari lhotari commented Nov 23, 2020

Copy link
Copy Markdown
Contributor

Motivation

Add support for using JDBC based device & tenant registry in the Hono Helm chart.
JDBC based device registries will be part of the upcoming Hono 1.5.0 release.

Fixes #163

Dependencies

@lhotari lhotari force-pushed the lh-hono-device-registry-jdbc branch 2 times, most recently from 3577b3a to 821f985 Compare November 23, 2020 16:35
@lhotari lhotari force-pushed the lh-hono-device-registry-jdbc branch 2 times, most recently from c47c934 to ba67aa6 Compare November 23, 2020 16:50

@ctron ctron left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me. But I will leave it for others to comment as well.

Comment thread charts/hono/Chart.yaml Outdated
Comment thread charts/hono/Chart.yaml Outdated
@kaniyan

kaniyan commented Nov 24, 2020

Copy link
Copy Markdown
Contributor

Looks good to me. With this PR, we can use this chart to deploy the JDBC device registry and connect to an already existing JDBC database. In order to provide an out of the box experience, I think its good to have an option to install a JDBC database instance on the fly. And the JDBC registry can connect to it as we do in the MongoDB device registry. I remember that in some meeting we talked about using a H2 database instance for that. Also it can be done as a separate PR.

@lhotari lhotari force-pushed the lh-hono-device-registry-jdbc branch from ecaef9e to 2fd9e33 Compare November 24, 2020 11:49
Comment thread charts/hono/values.yaml
Comment thread charts/hono/values.yaml
username:
# The password to use for authenticating to the database
password:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it would be nice to have default configuration for using an in memory H2 DB so that you can simply start the registry and use it, without the need to set a lot of configuration properties ...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

point taken, however setting up h2 using a persistent file would require quite some changes to the helm chart. Would in-memory h2 be ok?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sure, we can still evolve from there ...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am not sure I get the issue with persistence. That would be one pvc and a volume mapping in case of h2. Could be a simple feature flag to enable that.

@calohmn

calohmn commented Nov 25, 2020

Copy link
Copy Markdown
Contributor

@lhotari Can you rebase and resolve the conflict?
EDIT: seeing that this PR has to wait for the Hono 1.5.0 release, you can also do that later, as there will probably be some more updates to the Chart.yaml until then.

@lhotari lhotari force-pushed the lh-hono-device-registry-jdbc branch from 2fd9e33 to 550ae06 Compare November 25, 2020 14:40
@lhotari

lhotari commented Nov 25, 2020

Copy link
Copy Markdown
Contributor Author

Can you rebase and resolve the conflict?

@calohmn yes, that's done now.

@calohmn calohmn added the Hono label Dec 15, 2020
@sophokles73

Copy link
Copy Markdown
Contributor

hi @lhotari we have released Hono 1.5.0 today and it would be cool if we could add your changes now to the Helm chart. Would you mind rebasing your PR and bumping the chart version?

@lhotari lhotari force-pushed the lh-hono-device-registry-jdbc branch from 550ae06 to 85d4287 Compare December 17, 2020 12:18
@lhotari

lhotari commented Dec 17, 2020

Copy link
Copy Markdown
Contributor Author

hi @lhotari we have released Hono 1.5.0 today and it would be cool if we could add your changes now to the Helm chart. Would you mind rebasing your PR and bumping the chart version?

@sophokles73 I have rebased my PR and bumped the chart version to 1.4.17 . Is that correct?

@sophokles73 sophokles73 merged commit 4ec66de into eclipse-packages:master Dec 17, 2020
@sophokles73

Copy link
Copy Markdown
Contributor

@lhotari thanks for contributing 👍

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[eclipse/hono#2159] Add support to deploy JDBC based device registry

5 participants