Skip to content

Conversation

seung-00
Copy link
Contributor

What is this PR for?

This PR removes usage of the configuration API via WebSocket in the frontend.
The configuration API exists in both WS and REST, but unlike other APIs, it doesn’t seem to need two separate channels.
Maintaining both surfaces increases maintenance cost, so this PR unifies them by keeping only the REST API.

What type of PR is it?

Improvement

Todos

What is the Jira issue?

How should this be tested?

  • Check the configuration page (/configuration)
  • Check the notebook page (/notebook/{notebook_id})

Screenshots (if appropriate)

Questions:

  • Does the license files need to update? M
  • Is there breaking changes for older versions? N
  • Does this needs documentation? N

@seung-00 seung-00 force-pushed the feature/ZEPPELIN-6312 branch 4 times, most recently from f35d4eb to 4c00dde Compare August 31, 2025 16:28
Copy link
Contributor

@tbonelee tbonelee left a comment

Choose a reason for hiding this comment

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

Replacing WS with REST API seems reasonable since it reduce complexity.
As the login page is currently not working properly, that issue should be fixed as well.

@seung-00 seung-00 force-pushed the feature/ZEPPELIN-6312 branch from 4c00dde to 325c067 Compare September 6, 2025 07:32
@seung-00
Copy link
Contributor Author

seung-00 commented Sep 6, 2025

I’ve updated according to the review comments.
But I found one issue: the configurations WS API has no auth check, while the REST API does. So users without permission can still access the configs via WS.

Removing WS right now would break things, so I’ll fix this in another PR before merging this one.

@tbonelee

@tbonelee
Copy link
Contributor

tbonelee commented Sep 6, 2025

image Could you also take a look at this? It seems that fetching configurations in `ConfigurationService.initialize()` when the user is not logged in might be causing some issues.

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.

2 participants