Copilot commented on code in PR #13267:
URL: https://github.com/apache/ignite/pull/13267#discussion_r3542015265
##########
parent/pom.xml:
##########
@@ -683,6 +684,18 @@
<requireMavenVersion>
<version>[3.9.6,)</version>
</requireMavenVersion>
+ <bannedDependencies>
+ <excludes>
+ <exclude>javax.transaction:jta</exclude>
+
<exclude>javax.transaction:javax.transaction-api</exclude>
+
<exclude>jakarta.transaction:jakarta.transaction-api:(,2.0.0)</exclude>
Review Comment:
The bannedDependencies rule message says only jakarta.transaction-api 2.0.1+
is allowed, but the exclude currently bans versions <2.0.0, which would still
allow 2.0.0. Update the banned version range to match the message/intent.
##########
modules/rest-http/src/main/java/org/apache/ignite/internal/processors/rest/protocols/http/jetty/WelcomeHandler.java:
##########
@@ -75,7 +75,6 @@ else if (target.startsWith("/logo.svg")) {
res.getOutputStream().flush();
res.setStatus(HttpServletResponse.SC_OK);
- req.setHandled(true);
}
Review Comment:
The response status is set after writing and flushing the body. In a servlet
container the response may already be committed by then, so this call can be
ineffective/misleading. Set the status before writing (or drop the explicit
setStatus since 200 is the default).
##########
modules/core/src/main/java/org/apache/ignite/internal/processors/rest/GridRestProcessor.java:
##########
@@ -1017,6 +1017,10 @@ private void startHttpProtocol() throws
IgniteCheckedException {
log.debug("Failed to initialize HTTP REST protocol (consider
adding ignite-rest-http " +
"module to classpath).");
}
+ catch (LinkageError e) {
+ U.warn(log, "Failed to initialize HTTP REST protocol (consider
adding ignite-rest-http " +
+ "module and its dependencies to classpath): " + e);
+ }
Review Comment:
U.warn supports passing a Throwable; concatenating the LinkageError into the
message drops the stack trace from the warning log and makes troubleshooting
missing/incorrect dependencies harder.
##########
modules/jta/src/test/java/org/apache/ignite/internal/processors/cache/GridJtaLifecycleAwareSelfTest.java:
##########
@@ -142,8 +142,8 @@ public static class TestTxFactory extends
GridAbstractLifecycleAwareSelfTest.Tes
/** {@inheritDoc} */
@Test
- @Override public void testLifecycleAware() throws Exception {
- // No-op, see anothre tests.
+ @Override public void testLifecycleAware() {
+ // No-op, see another tests.
Review Comment:
Grammar in comment: "see another tests" is incorrect English; consider "see
other tests".
--
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]
