Copilot commented on code in PR #3746:
URL: https://github.com/apache/texera/pull/3746#discussion_r2364419168
##########
core/dao/build.sbt:
##########
@@ -36,6 +36,79 @@ ThisBuild / conflictManager := ConflictManager.latestRevision
// Restrict parallel execution of tests to avoid conflicts
Global / concurrentRestrictions += Tags.limit(Tags.Test, 1)
+/////////////////////////////////////////////////////////////////////////////
+// JOOQ Code Generation
+/////////////////////////////////////////////////////////////////////////////
+
+// Define JOOQ generation task
+lazy val jooqGenerate = taskKey[Seq[File]]("Generate JOOQ sources")
+
+jooqGenerate := {
+ val log = streams.value.log
+ log.info("Generating JOOQ classes...")
+
+ try {
+ import com.typesafe.config.{Config, ConfigFactory, ConfigParseOptions}
+ import org.jooq.codegen.GenerationTool
+ import org.jooq.meta.jaxb.{Configuration, Jdbc}
+ import java.nio.file.{Files, Path}
+ import java.io.File
+
+ // Load jOOQ configuration XML (absolute path from DAO project)
+ val jooqXmlPath: Path =
+
baseDirectory.value.toPath.resolve("src").resolve("main").resolve("resources").resolve("jooq-conf.xml")
+ val jooqConfig: Configuration =
GenerationTool.load(Files.newInputStream(jooqXmlPath))
+
+ // Load storage.conf from the config project
+ val storageConfPath: Path = baseDirectory.value.toPath
+ .getParent
+ .resolve("config")
+ .resolve("src")
+ .resolve("main")
+ .resolve("resources")
+ .resolve("storage.conf")
+
+ val conf: Config = ConfigFactory
+ .parseFile(
+ new File(storageConfPath.toString),
+ ConfigParseOptions.defaults().setAllowMissing(false)
+ )
+ .resolve()
+
+ // Extract JDBC configuration
+ val jdbcConfig = conf.getConfig("storage.jdbc")
+
+ val jooqJdbcConfig = new Jdbc
+ jooqJdbcConfig.setDriver("org.postgresql.Driver")
+ // Skip all the query params, otherwise it will omit the "texera_db."
prefix on the field names.
+ jooqJdbcConfig.setUrl(jdbcConfig.getString("url").split('?').head)
+ jooqJdbcConfig.setUsername(jdbcConfig.getString("username"))
+ jooqJdbcConfig.setPassword(jdbcConfig.getString("password"))
+
+ jooqConfig.setJdbc(jooqJdbcConfig)
+
+ // Generate the code
+ GenerationTool.generate(jooqConfig)
+ log.info("JOOQ code generation completed successfully")
+
+ // Return the generated files
+ val generatedDir = baseDirectory.value / "src" / "main" / "scala" / "edu"
/ "uci" / "ics" / "texera" / "dao" / "jooq" / "generated"
Review Comment:
The hardcoded package path should be extracted to a configuration variable
or derived from the JOOQ configuration to avoid duplication and improve
maintainability.
```suggestion
val packageName = jooqConfig.getGenerator.getTarget.getPackageName
val generatedDir = packageName
.split('.')
.foldLeft(baseDirectory.value / "src" / "main" / "scala")(_ / _)
```
##########
core/dao/build.sbt:
##########
@@ -36,6 +36,79 @@ ThisBuild / conflictManager := ConflictManager.latestRevision
// Restrict parallel execution of tests to avoid conflicts
Global / concurrentRestrictions += Tags.limit(Tags.Test, 1)
+/////////////////////////////////////////////////////////////////////////////
+// JOOQ Code Generation
+/////////////////////////////////////////////////////////////////////////////
+
+// Define JOOQ generation task
+lazy val jooqGenerate = taskKey[Seq[File]]("Generate JOOQ sources")
+
+jooqGenerate := {
+ val log = streams.value.log
+ log.info("Generating JOOQ classes...")
+
+ try {
+ import com.typesafe.config.{Config, ConfigFactory, ConfigParseOptions}
+ import org.jooq.codegen.GenerationTool
+ import org.jooq.meta.jaxb.{Configuration, Jdbc}
+ import java.nio.file.{Files, Path}
+ import java.io.File
+
+ // Load jOOQ configuration XML (absolute path from DAO project)
+ val jooqXmlPath: Path =
+
baseDirectory.value.toPath.resolve("src").resolve("main").resolve("resources").resolve("jooq-conf.xml")
+ val jooqConfig: Configuration =
GenerationTool.load(Files.newInputStream(jooqXmlPath))
+
+ // Load storage.conf from the config project
+ val storageConfPath: Path = baseDirectory.value.toPath
+ .getParent
+ .resolve("config")
+ .resolve("src")
+ .resolve("main")
+ .resolve("resources")
+ .resolve("storage.conf")
+
+ val conf: Config = ConfigFactory
+ .parseFile(
+ new File(storageConfPath.toString),
+ ConfigParseOptions.defaults().setAllowMissing(false)
+ )
+ .resolve()
+
+ // Extract JDBC configuration
+ val jdbcConfig = conf.getConfig("storage.jdbc")
+
+ val jooqJdbcConfig = new Jdbc
+ jooqJdbcConfig.setDriver("org.postgresql.Driver")
+ // Skip all the query params, otherwise it will omit the "texera_db."
prefix on the field names.
+ jooqJdbcConfig.setUrl(jdbcConfig.getString("url").split('?').head)
+ jooqJdbcConfig.setUsername(jdbcConfig.getString("username"))
+ jooqJdbcConfig.setPassword(jdbcConfig.getString("password"))
+
+ jooqConfig.setJdbc(jooqJdbcConfig)
+
+ // Generate the code
+ GenerationTool.generate(jooqConfig)
+ log.info("JOOQ code generation completed successfully")
+
+ // Return the generated files
+ val generatedDir = baseDirectory.value / "src" / "main" / "scala" / "edu"
/ "uci" / "ics" / "texera" / "dao" / "jooq" / "generated"
+ if (generatedDir.exists()) {
+ (generatedDir ** "*.java").get ++ (generatedDir ** "*.scala").get
+ } else {
+ Seq.empty
+ }
+ } catch {
+ case e: Exception =>
+ log.warn(s"JOOQ code generation failed: ${e.getMessage}")
Review Comment:
The error handling catches all exceptions but only logs the message.
Consider logging the full stack trace for debugging purposes, especially during
development.
```suggestion
log.warn(s"JOOQ code generation failed: ${e.getMessage}")
log.warn("JOOQ code generation failed with exception:", e)
```
##########
core/dao/build.sbt:
##########
@@ -36,6 +36,79 @@ ThisBuild / conflictManager := ConflictManager.latestRevision
// Restrict parallel execution of tests to avoid conflicts
Global / concurrentRestrictions += Tags.limit(Tags.Test, 1)
+/////////////////////////////////////////////////////////////////////////////
+// JOOQ Code Generation
+/////////////////////////////////////////////////////////////////////////////
+
+// Define JOOQ generation task
+lazy val jooqGenerate = taskKey[Seq[File]]("Generate JOOQ sources")
+
+jooqGenerate := {
+ val log = streams.value.log
+ log.info("Generating JOOQ classes...")
+
+ try {
+ import com.typesafe.config.{Config, ConfigFactory, ConfigParseOptions}
+ import org.jooq.codegen.GenerationTool
+ import org.jooq.meta.jaxb.{Configuration, Jdbc}
+ import java.nio.file.{Files, Path}
+ import java.io.File
+
+ // Load jOOQ configuration XML (absolute path from DAO project)
+ val jooqXmlPath: Path =
+
baseDirectory.value.toPath.resolve("src").resolve("main").resolve("resources").resolve("jooq-conf.xml")
+ val jooqConfig: Configuration =
GenerationTool.load(Files.newInputStream(jooqXmlPath))
+
+ // Load storage.conf from the config project
+ val storageConfPath: Path = baseDirectory.value.toPath
+ .getParent
+ .resolve("config")
+ .resolve("src")
+ .resolve("main")
+ .resolve("resources")
+ .resolve("storage.conf")
+
+ val conf: Config = ConfigFactory
+ .parseFile(
+ new File(storageConfPath.toString),
+ ConfigParseOptions.defaults().setAllowMissing(false)
+ )
+ .resolve()
+
+ // Extract JDBC configuration
+ val jdbcConfig = conf.getConfig("storage.jdbc")
+
+ val jooqJdbcConfig = new Jdbc
+ jooqJdbcConfig.setDriver("org.postgresql.Driver")
Review Comment:
The hardcoded PostgreSQL driver string should be extracted from
configuration or made configurable to support different database types in the
future.
```suggestion
jooqJdbcConfig.setDriver(jdbcConfig.getString("driver"))
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
