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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
