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

Enhanced WebUI #4406

Merged
merged 16 commits into from
Sep 12, 2024
Merged

Enhanced WebUI #4406

merged 16 commits into from
Sep 12, 2024

Conversation

wdbaruni
Copy link
Member

@wdbaruni wdbaruni commented Sep 11, 2024

Improving the embedding web ui and making it compatible with our latest APIs.

How to test locally

# this will build the static web ui and the binary as well
make build

# using the built binary
bacalhau serve --orchestrator --compute  -c WebUI.Enable

# now the webui can be reached at http://localhost:8438/

Main changes in core bacalhau

  • Enable CORS for websockets as well to enable streaming logs
  • Fix swagger generation for some APIs
    @udsamani if you can please review these changes

Main changes in the ui http server

  • Better handling for 404 instead of always returning the home page
  • Previously we were mutating the html files by replacing template placeholders with the http endpoint of the orchestrator node to allow the frontend to call the right node. Now the webui server exposes a /_config endpoint that the frontend calls when it loads to figure out the endpoints it needs to call
    @frrist if you can please review these changes. Just the webui/webui.go file

Main changes in the frontend

  • Frontend design close to figma https://www.figma.com/design/PAtQCqnpJ78UCM4oKHT2Y1/expanso?node-id=0-1&t=EoXg2e4UAWX4MsQv-0
  • Switched to nextjs and shadcn mainly to be consistent with Expanso Cloud implementation to enable code and component reuse
  • Used generated swagger models and APIs instead of manually creating new models
  • Support for streaming job logs
  • Bunch of other improvements
    @aronchick a lot of code changes here, but are mainly generated code by openapi and other tools we are using. Still I would appreciate overall feedback on the component's implementations as I am not an expert in FE coding

Remaining work for follow up PRs

  • Better error handling for failed API requests
  • Server disconnection handling and better reporting
  • Download job results
  • Job submission
  • Node management (approve, delete, ...)
  • Executions list and details view
  • Implement tests

Open questions

  • Storybook was dropped to release this faster. Do we think it is valuable to add in the future? @aronchick

Screenshots

image

image

image

image

image

@wdbaruni wdbaruni marked this pull request as ready for review September 11, 2024 22:02
Comment on lines +17 to +21
var WebsocketUpgrader = websocket.Upgrader{
CheckOrigin: func(r *http.Request) bool {
// TODO: make this more restricted or configurable
return true
},
Copy link
Member Author

Choose a reason for hiding this comment

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

needed for streaming logs. previously we only enabled CORS for all APIs except websockets. We need to make this configurable or more restricted in the future

@wdbaruni wdbaruni changed the title Enchaned WebUI Enhanced WebUI Sep 11, 2024
@wdbaruni wdbaruni self-assigned this Sep 11, 2024
Copy link
Collaborator

@udsamani udsamani left a comment

Choose a reason for hiding this comment

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

Swagger API generation LGTM

@@ -1,81 +1,234 @@
package webui
Copy link
Member

Choose a reason for hiding this comment

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

looks good to me

@wdbaruni wdbaruni merged commit 9f7486e into main Sep 12, 2024
4 checks passed
@wdbaruni wdbaruni deleted the web-ui branch September 12, 2024 22:57
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