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

[KYUUBI #6299] Support disabling Web UI #6311

Closed
wants to merge 3 commits into from

Conversation

lsm1
Copy link
Contributor

@lsm1 lsm1 commented Apr 15, 2024

🔍 Description

Issue References 🔗

This pull request fixes #6299

Describe Your Solution 🔧

when disabling web ui, return 404 page

Types of changes 🔖

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Test Plan 🧪

Behavior Without This Pull Request ⚰️

Behavior With This Pull Request 🎉

Related Unit Tests


Checklist 📝

Be nice. Be informative.

@@ -116,6 +116,13 @@ class KyuubiRestFrontendService(override val serverable: Serverable)
}

private def installWebUI(): Unit = {
if (!conf.get(FRONTEND_REST_ENABLE_WEBUI)) {
Copy link
Member

Choose a reason for hiding this comment

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

prefer to judge on the caller's side to make this function clear. BTW, what will happen if we don't inject the enable.html page?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we don't inject the enable.html, we just get a 404 page.

@cxzl25 cxzl25 changed the title [KYUUBI #6299]Support disabling Web UI [KYUUBI #6299] Support disabling Web UI Apr 16, 2024
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 85.71429% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 58.43%. Comparing base (cefc27f) to head (aa96c27).
Report is 6 commits behind head on master.

Files Patch % Lines
...ache/kyuubi/server/KyuubiRestFrontendService.scala 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #6311      +/-   ##
============================================
- Coverage     58.51%   58.43%   -0.08%     
  Complexity       24       24              
============================================
  Files           652      653       +1     
  Lines         39649    39775     +126     
  Branches       5454     5473      +19     
============================================
+ Hits          23199    23243      +44     
- Misses        13970    14039      +69     
- Partials       2480     2493      +13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pan3793 pan3793 added this to the v1.10.0 milestone Apr 16, 2024
@pan3793 pan3793 closed this in 11343cd Apr 16, 2024
@pan3793
Copy link
Member

pan3793 commented Apr 16, 2024

Thanks, merged to master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TASK][EASY] Support disabling Web UI
3 participants