gemmellr commented on code in PR #5149:
URL: https://github.com/apache/activemq-artemis/pull/5149#discussion_r1718275270
##########
artemis-web/pom.xml:
##########
@@ -59,8 +59,8 @@
<artifactId>artemis-commons</artifactId>
</dependency>
<dependency>
- <groupId>org.eclipse.jetty.toolchain</groupId>
- <artifactId>jetty-servlet-api</artifactId>
+ <groupId>jakarta.servlet</groupId>
+ <artifactId>jakarta.servlet-api</artifactId>
Review Comment:
Similarly
##########
pom.xml:
##########
@@ -105,10 +114,9 @@
<fuse.mqtt.client.version>1.16</fuse.mqtt.client.version>
<caffeine.version>3.1.8</caffeine.version>
<guava.version>33.2.1-jre</guava.version>
- <hawtio.version>2.17.7</hawtio.version>
+ <hawtio.version>4.0.0-RC1</hawtio.version>
Review Comment:
The console build is using 4.0.0 it appears, so seems like this should be
updated to at least that? (There is also a 4.1.0 now)
##########
tests/smoke-tests/src/main/filtered-resources/servers/linuxUpgradeETC/artemis.profile:
##########
@@ -33,7 +33,7 @@
ARTEMIS_INSTANCE_ETC_URI='file:${project.basedir}/target/classes/servers/linuxUp
# Hawtio Properties
# HAWTIO_ROLE define the user role or roles required to be able to login to
the console. Multiple roles to allow can
# be separated by a comma. Set to '*' or an empty value to disable role
checking when Hawtio authenticates a user.
-HAWTIO_ROLE='amq'
+HAWTIO_ROLES='amq'
Review Comment:
This (and other nearby configs) shouldnt change in this way, the aim is to
check the upgrade takes this old config and updates it to output the new
config, then check it for expectations directly and through comparing with a
freshly created broker instance config. These changes just put the new config
into the not-upgraded files.
##########
artemis-pom/pom.xml:
##########
@@ -524,10 +524,10 @@
<!-- License: (Joint): Apache 2.0 or EPL 2.0 -->
</dependency>
<dependency>
- <groupId>org.eclipse.jetty.toolchain</groupId>
- <artifactId>jetty-servlet-api</artifactId>
- <version>${jetty-servlet-api.version}</version>
- <!-- License: (Joint): Apache 2.0 or EPL 2.0 -->
+ <groupId>jakarta.servlet</groupId>
+ <artifactId>jakarta.servlet-api</artifactId>
+ <version>${jakarta.servlet-api.version}</version>
+ <!-- License: EPL 2.0 -->
Review Comment:
Related to earlier comment, think it should be left at the jetty one (now
jetty-jakarta-servlet-api to match the newer jetty).
##########
pom.xml:
##########
@@ -29,6 +29,14 @@
<relativePath>org.apache:apache</relativePath>
</parent>
+ <repositories>
+ <repository>
+ <name>Apache Staging Repository</name>
+ <id>apache.staging</id>
+ <url>https://repository.apache.org/content/groups/staging/</url>
+ </repository>
+ </repositories>
Review Comment:
Would configure this for releases-only so that it isnt ever checked for
snapshots (should anyone use this overall change against development commits at
a later point, it likely will be)
Maybe add a TODO:delete comment to make super clear it is temp and easy to
find later.
##########
.gitignore:
##########
@@ -33,3 +33,15 @@
artemis-native/src/main/c/org_apache_activemq_artemis_jlibaio_LibaioContext.h
# overlay outpit
**/overlays/**/*
+
+#for the console and yarn builds
+artemis-hawtio/artemis-plugin/artemis-plugin/build
+artemis-hawtio/artemis-plugin/artemis-plugin/node_modules
+artemis-hawtio/artemis-plugin/artemis-plugin/node
+# yarn (zero-installs)
+artemis-hawtio/artemis-plugin/artemis-plugin/.yarn/*
+artemis-hawtio/artemis-plugin/artemis-plugin/!.yarn/patches
+artemis-hawtio/artemis-plugin/artemis-plugin/!.yarn/plugins
+artemis-hawtio/artemis-plugin/artemis-plugin/!.yarn/releases
+artemis-hawtio/artemis-plugin/artemis-plugin/!.yarn/sdks
+artemis-hawtio/artemis-plugin/artemis-plugin/!.yarn/versions
Review Comment:
With the console bits in their own repo now, these shouldn't be needed any
more right?
##########
artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Upgrade.java:
##########
@@ -145,7 +147,7 @@ public Object run(ActionContext context) throws Exception {
"<env name=\"ARTEMIS_INSTANCE\"", "<env
name=\"ARTEMIS_INSTANCE_ETC\"",
"<env name=\"ARTEMIS_INSTANCE_URI\"", "<env
name=\"ARTEMIS_INSTANCE_ETC_URI\"",
"<env name=\"ARTEMIS_DATA_DIR\"", "<logpath>",
"<startargument>-Xmx", "<stopargument>-Xmx",
- "<name>", "<id>", "<startargument>-Dhawtio.role=");
+ "<name>", "<id>", "<startargument>-Dhawtio.roles=");
Review Comment:
This and the other related changes will only mean it keeps the _new_ config
for _future_ upgrades, after people have already upgraded. There dont appear to
be any changes here to make the upgrade keep the value from the original
pre-change configs, so it wont.
In addition to the hack of the pre-upgrade config I see you made (commented
on later), the related testing possibly also wont fail currently because we
didnt mess with this value previously, and so the existing test config is
perhaps using the default value, as will the new current one created for
comparison, so they will both have the same expected final output. The
test+config and then this upgrade will need modified to account for this
setting being manipulated so that it actually gets upgraded and verified to do
so.
##########
artemis-console/pom.xml:
##########
@@ -21,40 +21,36 @@
<parent>
<groupId>org.apache.activemq</groupId>
- <artifactId>artemis-hawtio-pom</artifactId>
+ <artifactId>artemis-pom</artifactId>
<version>2.36.0</version>
+ <relativePath>../artemis-pom/pom.xml</relativePath>
</parent>
<artifactId>artemis-console</artifactId>
<name>ActiveMQ Artemis Console</name>
<packaging>war</packaging>
- <properties>
- <activemq.basedir>${project.basedir}/../..</activemq.basedir>
- </properties>
-
<dependencies>
<dependency>
- <groupId>org.eclipse.jetty.toolchain</groupId>
- <artifactId>jetty-servlet-api</artifactId>
+ <groupId>jakarta.servlet</groupId>
+ <artifactId>jakarta.servlet-api</artifactId>
Review Comment:
This used the jetty dep before as we consolidated on it to avoid having
effectively duplicate bits in the distribution and the build. This will cause
that again. Not sure it needs changing other than adjusting to the newer jetty
releases dep (jetty-jakarta-servlet-api).
##########
artemis-console/pom.xml:
##########
@@ -21,40 +21,36 @@
<parent>
<groupId>org.apache.activemq</groupId>
- <artifactId>artemis-hawtio-pom</artifactId>
+ <artifactId>artemis-pom</artifactId>
<version>2.36.0</version>
+ <relativePath>../artemis-pom/pom.xml</relativePath>
</parent>
<artifactId>artemis-console</artifactId>
<name>ActiveMQ Artemis Console</name>
<packaging>war</packaging>
- <properties>
- <activemq.basedir>${project.basedir}/../..</activemq.basedir>
- </properties>
-
<dependencies>
<dependency>
- <groupId>org.eclipse.jetty.toolchain</groupId>
- <artifactId>jetty-servlet-api</artifactId>
+ <groupId>jakarta.servlet</groupId>
+ <artifactId>jakarta.servlet-api</artifactId>
<scope>provided</scope>
</dependency>
<dependency>
- <groupId>io.hawt</groupId>
- <artifactId>hawtio-war</artifactId>
- <version>${hawtio.version}</version>
+ <groupId>org.apache.activemq</groupId>
+ <artifactId>artemis-console-war</artifactId>
<type>war</type>
</dependency>
<!-- lets mark dependencies from the WAR as provided to avoid jetty:run
adding duplicates -->
- <dependency>
+ <!--<dependency>
Review Comment:
Can remove rather than comment out
--
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: gitbox-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
