gemini-code-assist[bot] commented on code in PR #39061:
URL: https://github.com/apache/beam/pull/39061#discussion_r3455687448


##########
sdks/java/testing/tpcds/build.gradle:
##########
@@ -34,6 +34,17 @@ def tpcdsRunnerProperty = "tpcds.runner"
 def tpcdsRunnerDependency = project.findProperty(tpcdsRunnerProperty)
         ?: ":runners:direct-java"
 def isSpark = tpcdsRunnerDependency.startsWith(":runners:spark:")
+def isFlink2 = tpcdsRunnerDependency.startsWith(":runners:flink:") && {
+  def version = tpcdsRunnerDependency.substring(":runners:flink:".length())
+  try {
+    def parts = version.split('\\.')
+    def major = parts[0].toInteger()
+    def minor = parts.length > 1 ? parts[1].toInteger() : 0
+    return (major > 2) || (major == 2 && minor >= 1)
+  } catch (Exception e) {
+    return false
+  }
+}()

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The current version parsing logic is quite verbose and uses a `try-catch` 
block to handle potential parsing errors. We can simplify this significantly 
and make it more robust against null values or unexpected formats by using a 
regular expression matcher.
   
   ```
   def flinkMatcher = tpcdsRunnerDependency ? (tpcdsRunnerDependency =~ 
/:runners:flink:(\d+)\.(\d+)/) : null
   def isFlink2 = flinkMatcher?.find() && {
     def major = flinkMatcher.group(1).toInteger()
     def minor = flinkMatcher.group(2).toInteger()
     (major > 2) || (major == 2 && minor >= 1)
   }()
   ```



##########
sdks/java/testing/nexmark/build.gradle:
##########
@@ -39,6 +39,17 @@ def nexmarkRunnerDependency = 
project.findProperty(nexmarkRunnerProperty)
 def nexmarkRunnerVersionProperty = "nexmark.runner.version"
 def nexmarkRunnerVersion = project.findProperty(nexmarkRunnerVersionProperty)
 def isSparkRunner = nexmarkRunnerDependency.startsWith(":runners:spark:")
+def isFlink2 = nexmarkRunnerDependency.startsWith(":runners:flink:") && {
+  def version = nexmarkRunnerDependency.substring(":runners:flink:".length())
+  try {
+    def parts = version.split('\\.')
+    def major = parts[0].toInteger()
+    def minor = parts.length > 1 ? parts[1].toInteger() : 0
+    return (major > 2) || (major == 2 && minor >= 1)
+  } catch (Exception e) {
+    return false
+  }
+}()

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The current version parsing logic is quite verbose and uses a `try-catch` 
block to handle potential parsing errors. We can simplify this significantly 
and make it more robust against null values or unexpected formats by using a 
regular expression matcher.
   
   ```
   def flinkMatcher = nexmarkRunnerDependency ? (nexmarkRunnerDependency =~ 
/:runners:flink:(\d+)\.(\d+)/) : null
   def isFlink2 = flinkMatcher?.find() && {
     def major = flinkMatcher.group(1).toInteger()
     def minor = flinkMatcher.group(2).toInteger()
     (major > 2) || (major == 2 && minor >= 1)
   }()
   ```



-- 
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]

Reply via email to