exceptionfactory commented on code in PR #10249:
URL: https://github.com/apache/nifi/pull/10249#discussion_r2350504494
##########
nifi-system-tests/nifi-system-test-suite/src/test/java/org/apache/nifi/tests/system/clustering/FlowSynchronizationIT.java:
##########
@@ -606,9 +606,14 @@ public void
testAddControllerServiceReferencingExistingService() throws NiFiClie
// Delete the CountFlowFiles processor, and countB and countC
services, disable A.
getClientUtil().stopProcessor(countFlowFiles);
+ // Ensure the processor has fully stopped before disabling services to
avoid race conditions
+ getClientUtil().waitForStoppedProcessor(countFlowFiles.getId());
Review Comment:
Should these changes be handled separately?
##########
nifi-registry/pom.xml:
##########
@@ -35,6 +35,10 @@
<module>nifi-registry-docker-maven</module>
</modules>
<properties>
+ <nifi.registry.jetty.version>12.0.25</nifi.registry.jetty.version>
Review Comment:
Is there a reason to introduce this new property as opposed to overriding
the value from the parent Maven configuration?
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/JettyServer.java:
##########
@@ -679,13 +673,13 @@ private void addDocsServlets(WebAppContext webAppContext)
{
try {
final File docsDir = getDocsDir();
- final ServletHolder docs = new ServletHolder("docs",
ResourceServlet.class);
+ final ServletHolder docs = new ServletHolder("docs",
DefaultServlet.class);
final Path htmlBaseResource = docsDir.toPath().resolve("html");
docs.setInitParameter("baseResource", htmlBaseResource.toString());
docs.setInitParameter("dirAllowed", "false");
webAppContext.addServlet(docs, "/html/*");
- final ServletHolder restApi = new ServletHolder("rest-api",
ResourceServlet.class);
+ final ServletHolder restApi = new ServletHolder("rest-api",
DefaultServlet.class);
Review Comment:
Is there a particular reason for changing from the ResourceServlet to the
DefaultServlet? This previously resulted in warnings related to the
DefaultServlet and path configuration.
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/src/main/java/org/apache/nifi/web/server/StandardServerProvider.java:
##########
@@ -63,7 +63,7 @@ class StandardServerProvider implements ServerProvider {
public Server getServer(final NiFiProperties properties) {
Objects.requireNonNull(properties, "Properties required");
- final QueuedThreadPool threadPool = new
QueuedThreadPool(properties.getWebThreads());
+ final VirtualThreadPool threadPool = new
VirtualThreadPool(properties.getWebThreads());
Review Comment:
I recommend leaving this as the `QueuedThreadPool` for now, because the web
threads property is less meaningful with Virtual Threads. It may be worth
considering deprecating it. Either way, it seems like something better to
consider in a separate issue.
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-jetty/pom.xml:
##########
@@ -85,6 +85,25 @@
<artifactId>nifi-web-servlet-shared</artifactId>
<version>2.6.0-SNAPSHOT</version>
</dependency>
+ <dependency>
+ <groupId>org.eclipse.jetty</groupId>
+ <artifactId>jetty-deploy</artifactId>
Review Comment:
The `jetty-deploy` library brings in several other dependencies. Is there a
reason this is necessary in Jetty 12.1 and not 12.0?
--
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]
