Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Artemis Console next gen based on hawtIO 4 #5149

Closed
wants to merge 9 commits into from

Conversation

andytaylor
Copy link
Contributor

@andytaylor andytaylor commented Aug 15, 2024

This is a PR to allow developers to build a variant of the latest Artemis Release with the new Artemis Console embedded, for testing. It will not currently merge and isnt intended to.

I have staged the 1st Artemis Console release candidate, 1.0.0, and this build will use the staged dependency when built.

I will make this PR a draft and update it for each Artemis Release until it is decided to merge into main.

@andytaylor andytaylor marked this pull request as draft August 15, 2024 08:46
Copy link
Member

@gemmellr gemmellr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looked over some of it (not the tests) and noted some small things

.gitignore Outdated Show resolved Hide resolved
artemis-console/pom.xml Outdated Show resolved Hide resolved
artemis-console/pom.xml Outdated Show resolved Hide resolved
artemis-pom/pom.xml Outdated Show resolved Hide resolved
artemis-web/pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
pom.xml Outdated Show resolved Hide resolved
@jbertram
Copy link
Contributor

jbertram commented Aug 15, 2024

Couple of things I noticed right off the bat...

  1. When the broker starts with the new console it logs this WARN:
    WARN  [io.hawt.web.ServletHelpers] Couldn't find file on location: file:///path/to/activemq-artemis/artemis-distribution/target/apache-artemis-2.36.0-bin/apache-artemis-2.36.0/bin/test/etc/hawtio-oidc.properties
    
  2. It looks like there's supposed to be an image on the log-in screen, but it just displays the text "Artemis Console"
    (image)
  3. It appears there's a similar image rendering problem once you log-in (image)
  4. The broker used to log the following when starting up with the previous console. It's worth keeping these, I think:
    INFO  [org.apache.activemq.artemis] AMQ241002: Artemis Jolokia REST API available at http://localhost:8161/console/jolokia
    INFO  [org.apache.activemq.artemis] AMQ241004: Artemis Console available at http://localhost:8161/console
    
  5. Could we use the old background on the log-in screen (i.e. this)? It jives more with ActiveMQ Artemis branding.
  6. Can you change references strictly to "Artemis" to be "ActiveMQ Artemis"?

@gemmellr
Copy link
Member

I dont recall either way about the first issue you listed (didnt spot it though if it was there), but I didnt notice the 2nd or 3rd. Seems like one of us possibly had something cached. I'll give it another build tomorrow and check. I did notice the 4th about the log messages but forgot to post it in my final summary comment.

@jbertram
Copy link
Contributor

@gemmellr, so I restarted the broker, opened the console again, and forced a complete reload and the images appeared properly. That takes care of issues 2, 3, & 5!

@gemmellr
Copy link
Member

Couple of things I noticed right off the bat...

1. When the broker starts with the new console it logs this `WARN`:
   ```
   WARN  [io.hawt.web.ServletHelpers] Couldn't find file on location: file:///path/to/activemq-artemis/artemis-distribution/target/apache-artemis-2.36.0-bin/apache-artemis-2.36.0/bin/test/etc/hawtio-oidc.properties
   ```

I do see this one too. It is logging from a hawtio class, and seems like it will go away with hawtio 4.1.0 as they switched it to debug:
hawtio/hawtio@406c681#diff-7dbf56158197c9c42c5cda82f9ce400be6b592378d509def40c0c98e7c823325

@clebertsuconic
Copy link
Contributor

I saw the exception.. I couldn't get the web console to run.. was I supposed to do anything to be able to run it?

@gemmellr
Copy link
Member

I saw the exception..

The warning log messaged from hawtio discussed before? Or Something else?

I couldn't get the web console to run.. was I supposed to do anything to be able to run it?

Just build + start the broker. When you say you couldnt get it to run, what exactly did you encounter? An error loading the console page, or did you just not try it because it didnt log its URL (which hasnt changed) at startup? Per earlier discussion, it wasnt logging the messages as before, but was still there.

Comment on lines -120 to -129
<dependency>
<groupId>org.apache.activemq</groupId>
<artifactId>activemq-branding</artifactId>
<type>war</type>
</dependency>
<dependency>
<groupId>org.apache.activemq</groupId>
<artifactId>artemis-plugin</artifactId>
<type>war</type>
</dependency>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is as much a comment on the unaltered bit remaining, as the removals.

The binary distribution assembly has a number of entries in its LICENCE file (and files it references) to do with covering the console bits. Those files haven't been changed here, but it seems almost certain they will need to be. That is, artemis-distribution/src/main/resources/licenses/bin/LICENSE and files under dir artemis-distribution/src/main/resources/licenses/bin/licenses/.

@gemmellr
Copy link
Member

I added some commits fixing many of the code items I commented on earlier. I also switched it to the current SNAPSHOT since the staging repo was dropped when the vote was cancelled, so the build wouldn't work at all otherwise.

Remaining open points are the licence stuff and other things discussed in the comments, e.g startup log messages / warnings.

@andytaylor
Copy link
Contributor Author

which references do you refer to @jbertram ?

  1. Can you change references strictly to "Artemis" to be "ActiveMQ Artemis"?

@andytaylor
Copy link
Contributor Author

closing and replaced by #5245

@andytaylor andytaylor closed this Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants