You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@texera.apache.org by "seongjinyoon (via GitHub)" <gi...@apache.org> on 2025/09/19 20:02:53 UTC

[PR] feat: automate JOOQ code generation in SBT build process [texera]

seongjinyoon opened a new pull request, #3746:
URL: https://github.com/apache/texera/pull/3746

   **Description:**
   Integrate JOOQ code generation into SBT build system
   
   **Changes:**
   Add JOOQ generation task using SBT source generators that run before compilation
   Add JOOQ dependencies required for build time code generation


-- 
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: dev-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat: automate JOOQ code generation in SBT build process [texera]

Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan merged PR #3746:
URL: https://github.com/apache/texera/pull/3746


-- 
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: dev-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat: automate JOOQ code generation in SBT build process [texera]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
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: dev-unsubscribe@texera.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org