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

Initial commit of React frontend #48

Merged

Conversation

DomGarguilo
Copy link

No description provided.

@DomGarguilo
Copy link
Author

In its current state, the homepage is just a test layout. The data that populates it is not real and the homepage will definitely change once we figure out what we want displayed there.

The "Resouce Groups" page is just a list to links of individual resource group pages at the moment.

Here is an individual RG page. In this case the default group:
image

Here is another example with a group that only contains tservers on these resource group pages, we show a summary of the metrics for each server type in that group:
image

On the individual server pages we just show the hostname, a link back to the resource group that the component is in, and a table of the metrics that correspond to that server.
image

@DomGarguilo
Copy link
Author

To run the new frontend, you can:

  1. stand up an instance on this branch, then cd into the root of this npm project folder and run npm run dev this will start the development build of the project.
  2. run npm run build before building accumulo. This will package up the code to be served by the backend on port 43331.

@DomGarguilo
Copy link
Author

Screenshot for the change 079fbd1:
image

@dlmarion dlmarion merged commit e7b1bb3 into dlmarion:4973-new-monitor-metrics Nov 9, 2024
8 checks passed
Copy link

@ctubbsii ctubbsii left a comment

Choose a reason for hiding this comment

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

I haven't tried to run this yet, but my gut reaction is that this seems like so much overkill for what we use the monitor for. We previously talked about getting rid of the monitor, and I have worked to simplify its maintenance and reduce its scope. This seems like heading the opposite direction, with more infrastructure built around maintaining it, to further entrench it in our build, rather than whittle it down and keep it a lightweight thing to maintain.

I realize that my gut reaction may also be driven by my overall lack of familiarity with these things... so I acknowledge that what seems like complexity to me may just be ignorance. Nevertheless, I don't want to completely discount my instinct on this, either. I would like to understand more about the value that is added by making our project become a react web project, before forming a more concrete opinion.

Comment on lines +283 to +285
<groupId>com.github.eirslett</groupId>
<artifactId>frontend-maven-plugin</artifactId>
<version>1.15.1</version>

Choose a reason for hiding this comment

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

I'm not a big fan of this plugin's downloading and installing npm. What we do for the javascript formatting is just run an external script that runs if npm is already installed. I think using the user's already-installed npm is better. We do the same with thrift... you have to install thrift yourself. For thrift, we put that part in a build profile, so if you don't have thrift installed, it's fine as long as you're not modifying the thrift bits. Here, it seems we could do the same, but perhaps we require this to run every time?

</configuration>
<executions>
<execution>
<id>install node and npm</id>

Choose a reason for hiding this comment

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

IDs should not have spaces

Choose a reason for hiding this comment

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

As this is primarily a Java project, not a web project, and because npm is plagued with supply chain issues, I have concerns that we aren't really qualified to maintain these dependencies for this, and keeping this up-to-date and secure with as many dependencies as it brings in, is going to be a burden.

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.

3 participants