Re: [PR] Deprecate Java 8 (druid)

["adarshsanjeev (via GitHub)" <gi...@apache.org>]

adarshsanjeev commented on code in PR #17466:
URL: https://github.com/apache/druid/pull/17466#discussion_r1841653525


##########
.github/workflows/revised-its.yml:
##########
@@ -86,8 +86,8 @@ jobs:
     uses: ./.github/workflows/reusable-revised-its.yml
     if: ${{ needs.changes.outputs.core == 'true' || needs.changes.outputs.common-extensions == 'true' }}
     with:
-      build_jdk: 8
-      runtime_jdk: 11
+      build_jdk: 11
+      runtime_jdk: 17

Review Comment:
   Just curious, why is the runtime higher?



##########
check_test_suite.py:
##########
@@ -22,7 +22,7 @@
 # this script does some primitive examination of git diff to determine if a test suite needs to be run or not
 
 # these jobs should always be run, no matter what
-always_run_jobs = ['license checks', '(openjdk8) packaging check', '(openjdk11) packaging check']
+always_run_jobs = ['license checks', '(openjdk11) packaging check']

Review Comment:
   I'm not familiar with this, but should we start running the check for 17 now?



##########
processing/src/main/java/org/apache/druid/query/rowsandcols/CursorFactoryRowsAndColumns.java:
##########
@@ -112,12 +112,8 @@ private static RowsAndColumns materialize(CursorFactory cursorFactory)
           cursor.advance();
         }
 
-        if (writer == null) {
-          return new EmptyRowsAndColumns();
-        } else {
-          final byte[] bytes = writer.toByteArray();
-          return new ColumnBasedFrameRowsAndColumns(Frame.wrap(bytes), rowSignature);
-        }
+        final byte[] bytes = writer.toByteArray();

Review Comment:
   Removing a null check for whatever reason seems weird. Is there a reason that it wasn't showing up in Java 8? If it was suppressed there, perhaps it should be here as well.



##########
server/src/main/java/org/apache/druid/curator/discovery/CuratorDruidLeaderSelector.java:
##########
@@ -58,7 +59,7 @@ public class CuratorDruidLeaderSelector implements DruidLeaderSelector
   private final AtomicReference<LeaderLatch> leaderLatch = new AtomicReference<>();
 
   private volatile boolean leader = false;
-  private volatile int term = 0;
+  private final AtomicInteger term = new AtomicInteger(0);

Review Comment:
   Just curious, what is the checkstyle rule that was failing here?



##########
.github/workflows/unit-and-integration-tests-unified.yml:
##########
@@ -162,7 +162,7 @@ jobs:
       fail-fast: false
       matrix:
         # Use JDK 21.0.4 to work around https://github.com/apache/druid/issues/17429
-        jdk: [ '11', '17', '21.0.4' ]
+        jdk: [ '17', '21.0.4' ]

Review Comment:
   Does 11 need to be removed here?



-- 
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: commits-unsubscribe@druid.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org