feat: extract databases in Kotlin schema #762
Conversation
worstell
commented
Jan 12, 2024
•
worstell
commented
worstell
requested review from
alecthomas,
dhanji and
wesbillman
as code owners
worstell
force-pushed
the
worstell/20240108-dbconn
branch
3 times, most recently
from
3f65e47
to
400e2db
Compare
integration/integration_test.go
Outdated
| @@ -221,6 +230,71 @@ func call[Resp any](module, verb string, req obj, onResponse func(t testing.TB, | |||
| } | |||
| } | |||
|
|
|||
| func setUpKotlinModuleDb(dir string) assertion { | |||
There was a problem hiding this comment.
This should just be setupModuleDb as it's not Kotlin specific.
There was a problem hiding this comment.
updated - right now this scaffolds the kotlin DbTest.kt file, but we can always follow up with a change to write language-appropriate content once Go is ready
| data class DbRequest(val data: String?) | ||
| data class DbResponse(val message: String? = "ok") | ||
|
|
||
| val db = Database("testdb") |
There was a problem hiding this comment.
This is awesome Lizzy.
| } | ||
|
|
||
| fun persistRequest(req: DbRequest) { | ||
| db.conn { |
There was a problem hiding this comment.
it turns out java.sql.Connection extends AutoCloseable, which handles resource management with the caveat that the close() logic is only properly utilized if you use this .use function
so now calling db.conn { ... } creates a new connection and executes the provided block in a .use function to make sure the resource is properly managed. also, not sure if this fixed the integration test but with a few other fixes it's working now!
| comments = module.comments() | ||
| ) | ||
| modules[currentModuleName]?.decls?.addAll(moduleData.decls) ?: run { | ||
| modules[currentModuleName] = moduleData | ||
| } | ||
| } | ||
|
|
||
| private fun extractDatabases(): Set<Decl> { |
|
I forgot to add: please add support to Go for this. We want to keep the two runtimes at feature parity. |
will do - was thinking i should prioritize the remaining work here first though, unless you think otherwise for now i can make tickets to add Go support for all the new Kotlin features (this + secrets/KMS APIs)? |
|
I'd prefer to keep both at parity in main at all times. We've let them get out of sync before and it ends up being a massive PITA bringing them up to date. If you want to prioritise the other work though, defer merging this PR until it supports Go. I'm happy to pair with you on this in the morning if you like. |
|
Actually disregard that, let's get this in. I'll get the Go side working while you work on the other Kotlin stuff. |
worstell
force-pushed
the
worstell/20240111-parse-database-schema
branch
from
b033e8e
to
36b57c7
Compare
worstell
force-pushed
the
worstell/20240111-parse-database-schema
branch
from
36b57c7
to
9a345a7
Compare
worstell
force-pushed
the
worstell/20240111-parse-database-schema
branch
from
9a345a7
to
241f0fe
Compare
