Skip to content

Add a basic frontend to the web plugin #2638

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

Merged
merged 49 commits into from
Nov 10, 2022

Conversation

tobim
Copy link
Member

@tobim tobim commented Oct 17, 2022

This PR adds a built-in web front end to the web plugin.

It can be opened at the root of the HTTP server.
For example with this in your vast.yaml:

vast:
  plugins: [ "all" ]
  start:
    print-endpoint: true
    commands:
      - web server

plugins:
  web:
    mode: dev
    bind: 127.0.0.1
    port: 5657

You can point your browser to http://127.0.0.1:5657 to find the app.

📝 Reviewer Checklist

Review this pull request by ensuring the following items:

  • All user-facing changes have changelog entries
  • User-facing changes are reflected on vast.io

@tobim tobim added the feature New functionality label Oct 17, 2022
@dit7ya
Copy link
Member

dit7ya commented Oct 17, 2022

Seems like pnpm needs to be installed in the CI environment? https://github.com/tenzir/vast/actions/runs/3265777637/jobs/5368525733#step:7:54

@tobim tobim force-pushed the story/sc-38451/integrate-frontend-web branch from b180379 to 9685f6a Compare October 17, 2022 15:29
@dit7ya
Copy link
Member

dit7ya commented Oct 18, 2022

I tried to locally build from this branch but
cmake -B build -DCMAKE_BUILD_TYPE=Release -DVAST_PLUGINS=plugins/web fails with

CMake Error at plugins/web/aux/CMakeLists.txt:20 (add_custom_command):
  add_custom_command Wrong syntax.  A TARGET or OUTPUT must be specified.

@lava
Copy link
Member

lava commented Oct 18, 2022

@dit7ya This is caused by a missing bundled restinio (the error message is pretty terrible tbh), it should be solved by git submodule update --init.

@mavam
Copy link
Member

mavam commented Oct 18, 2022

There are two vast.svg files added in this PR. Is this on purpose?

@dominiklohmann
Copy link
Member

@dit7ya This is caused by a missing bundled restinio (the error message is pretty terrible tbh), it should be solved by git submodule update --init.

Can you write a story for improving them? That should be easy enough to check ahead of assuming they exist.

tobim added 5 commits October 24, 2022 10:59
This commit migrates the state of tenzir/vast-frontend
as of c88b0f71d into the web plugin.
You need to call `scripts/git-setup.sh -f` to install the update.
@tobim tobim force-pushed the story/sc-38451/integrate-frontend-web branch from 189f2a3 to 755a3d5 Compare October 24, 2022 09:19
@netantho
Copy link
Contributor

netantho commented Oct 26, 2022

Update: Fixed by commit below :-)

When I don't have any record in VAST, I get the following error:

bodyRows.js?v=76b13408:124 Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'map')
    at getBodyRows (bodyRows.js?v=76b13408:124:23)
    at createViewModel.js?v=76b13408:12:16
    at sync (index.mjs:77:28)
    at index.mjs:95:9
    at Object.subscribe2 [as subscribe] (index.mjs:48:20)
    at subscribe (index.mjs:55:25)
    at index.mjs:85:62
    at Array.map (<anonymous>)
    at index.mjs:85:44
    at Object.subscribe2 [as subscribe] (index.mjs:48:20)

The UI shows the VAST server as not running where it's actually running, just in the CLI vast count returns 0 (try moving you're vast.db folder for example to get a new empty one)

I gets solved after importing a few records. It's so cool to have a webUI, even if it's just status page for now :-)

@dit7ya
Copy link
Member

dit7ya commented Oct 27, 2022

@netantho Thanks for the feedback. I have pushed commits which should fix them. Since the frontend is at the very basic state right now, a lot of things are yet to be implemented - which should prevent this kind of bugs.

@netantho
Copy link
Contributor

@netantho Thanks for the feedback. I have pushed commits which should fix them. Since the frontend is at the very basic state right now, a lot of things are yet to be implemented - which should prevent this kind of bugs.

Thanks @dit7ya! I confirm that your fixes for both of the bugs I found and reported :-)

@dit7ya
Copy link
Member

dit7ya commented Oct 27, 2022

I have figured out some issues in the CI for this PR and fixing them on #2656. I am going to push some of those changes, and also going to probably rebase this on latest master, if there is no objection.

@dit7ya dit7ya force-pushed the story/sc-38451/integrate-frontend-web branch from 5faaefa to fbd5857 Compare October 27, 2022 12:29
@lava lava force-pushed the story/sc-38451/integrate-frontend-web branch from 97079ee to ad9f7e4 Compare October 28, 2022 09:05
@lava lava force-pushed the story/sc-38451/integrate-frontend-web branch from ad9f7e4 to cbaf966 Compare October 28, 2022 09:13
Copy link
Member Author

@tobim tobim left a comment

Choose a reason for hiding this comment

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

I tested this locally and the static binary package now comes with a nice web ui! 🥳

One extra note because you can't comment on files in GitHub:
This adds lock files for both yarn and pnpm, do we really need both?

Copy link
Member

@dominiklohmann dominiklohmann left a comment

Choose a reason for hiding this comment

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

This is ready to ship in my opinion. I'd appreciate a second review from either @lava or @tobim, though, before merging this.

Copy link
Member

@lava lava left a comment

Choose a reason for hiding this comment

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

From my side this is good to go as well but I went ahead and also merged #2681 into this PR so we atomically have the SBOM changes when we add the frontend code.

Copy link
Member Author

@tobim tobim left a comment

Choose a reason for hiding this comment

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

This really deserves a changelog entry before it gets merged. @dit7ya do you want me to write one?

@dit7ya
Copy link
Member

dit7ya commented Nov 10, 2022

Yes, that would be great @tobim.

On a sepatate note, macOS CI still seems unhappy 😭.

@tobim
Copy link
Member Author

tobim commented Nov 10, 2022

Yes, that would be great @tobim.

On a sepatate note, macOS CI still seems unhappy sob.

I think this is an unrelated bug that somehow reached master: #2686 (comment)
EDIT: Dominik already pushed the fix. Thank you!

@dominiklohmann
Copy link
Member

I pushed a fix for that @tobim, see comment in that PR.

@dominiklohmann dominiklohmann merged commit 5dbc045 into master Nov 10, 2022
@dominiklohmann dominiklohmann deleted the story/sc-38451/integrate-frontend-web branch November 10, 2022 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants