-
Notifications
You must be signed in to change notification settings - Fork 8
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
feat: provide Kotlin API for db connections #753
Conversation
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.
Overall LGTM, but please change the config file to use an envar with a cross-language-compatible DSN format, and translate that into whatever Java supports in the runtime.
kotlin-runtime/ftl-runtime/src/main/kotlin/xyz/block/ftl/database/Db.kt
Outdated
Show resolved
Hide resolved
kotlin-runtime/ftl-runtime/src/main/kotlin/xyz/block/ftl/database/Db.kt
Outdated
Show resolved
Hide resolved
2d7176a
to
349d6b4
Compare
61db968
to
35d2773
Compare
3f65e47
to
400e2db
Compare
assert.NoError(t, err) | ||
|
||
var exists bool | ||
query := `SELECT EXISTS(SELECT datname FROM pg_catalog.pg_database WHERE datname = $1);` |
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.
Nice!
val host = uri.host ?: throw IllegalArgumentException("Missing host in DSN.") | ||
val port = if (uri.port != -1) uri.port.toString() else throw IllegalArgumentException("Missing port in DSN.") | ||
val database = uri.path.trimStart('/') | ||
val parameters = uri.query?.replace("&", "?") ?: "" |
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.
This makes me sad.
"connectrpc.com/connect" | ||
"github.com/alecthomas/assert/v2" | ||
_ "github.com/amacneil/dbmate/v2/pkg/driver/postgres" |
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.
What's this for?
Oh is this redundant now? |
yeah, had intended for these to be split into two PRs (this with the API implementation and the other with schema parsing), but i think since the latter was based on this branch and both commits showed up on github (probably needed to rebase to fix this) it was confusing. going to assume the stamp on the other PR includes these changes and close this 😄 |
schema parsing to follow