You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@zeppelin.apache.org by "seung-00 (via GitHub)" <gi...@apache.org> on 2025/10/08 09:52:23 UTC

[PR] [ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API [zeppelin]

seung-00 opened a new pull request, #5099:
URL: https://github.com/apache/zeppelin/pull/5099

   ### What is this PR for?
   Currently, configuration data is fetched through both REST API and WebSocket channels. However, the WebSocket path does not perform permission checks, and the only required data from it is the WebSocket max message size. 
   
   I extracted the websocket max message size field into a dedicated REST API, to improve security and simplify configuration handling.
   
   ### What type of PR is it?
   Improvement
   
   ### Todos
   * [ ] #5060
   
   ### What is the Jira issue?
   * Open an issue on Jira https://issues.apache.org/jira/browse/ZEPPELIN/6366
   
   ### 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? N
   * Is there breaking changes for older versions? N
   * Does this needs documentation? N
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API [zeppelin]

Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on PR #5099:
URL: https://github.com/apache/zeppelin/pull/5099#issuecomment-3382982792

   I don't know if we should also adapt zeppelin-web? What do you think?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API [zeppelin]

Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on code in PR #5099:
URL: https://github.com/apache/zeppelin/pull/5099#discussion_r2485794479


##########
zeppelin-server/src/main/java/org/apache/zeppelin/rest/ConfigurationsRestApi.java:
##########
@@ -32,7 +32,7 @@
 import org.apache.zeppelin.service.AuthenticationService;
 
 /** Configurations Rest API Endpoint. */
-@Path("/configurations")

Review Comment:
   Hi @seung-00 ,
   Thank you for bringing this issue to my attention again.
   
   As I see it, we have the following options.
   
   1) We include the path `/api/configurations/wsMaxMessageSize` in `shiro.ini` and give unauthenticated users the right to call this endpoint.
   This would be a breaking change.
   2) We add a new path, but it would also have to contain something related to configuration in the path, e.g., `/api/configuration/wsMaxMessageSize` (without `s`). The entire API endpoint would then also have to be mapped to a new class.
   The `shiro.ini` would also have to be adjusted here, and that would also be a breaking change.
   
   What bothers me about 1) is that potentially new frontend configuration options would also have to be added to the `shiro.ini`.
   What bothers me about 2) is that it's quite confusing, but you probably only need to adjust the `shiro.ini` once.
   
   What do you think about creating a general endpoint for frontend configurations? With this pull request, it's `wsMaxMessageSize`, tomorrow it might be something else. Then you could build the whole thing in the `ConfigurationsRestApi` and use `frontend-config` or something else as the endpoint path. You would only have to adjust `shiro.ini` once and extend the map that the endpoint returns.
   
   Either way, this change would be a breaking change, since the `shiro.ini`, for example, is very variable.
   See: https://github.com/apache/zeppelin/blob/master/conf/shiro.ini.template#L129-L130



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API [zeppelin]

Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on code in PR #5099:
URL: https://github.com/apache/zeppelin/pull/5099#discussion_r2431365878


##########
zeppelin-server/src/main/java/org/apache/zeppelin/rest/ConfigurationsRestApi.java:
##########
@@ -32,7 +32,7 @@
 import org.apache.zeppelin.service.AuthenticationService;
 
 /** Configurations Rest API Endpoint. */
-@Path("/configurations")

Review Comment:
   You have changed the path of the API class here. Probably so that the call is `/api/wsMaxMessageSize`. However, I also think `/api/configurations/wsMaxMessageSize` is very good and would prefer this API endpoint.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API [zeppelin]

Posted by "tbonelee (via GitHub)" <gi...@apache.org>.
tbonelee commented on PR #5099:
URL: https://github.com/apache/zeppelin/pull/5099#issuecomment-3384778554

   @seung-00 
   (If we also apply this to `zeppelin-web`.)
   There is a failing test in `zeppelin-integration`.
   After a quick note: in `zeppelin-web`'s `NoteImportCtrl`, `wsMaxMessageSize` is called even when the user is not logged in.
   The server then returns a 302 redirect to `/api/login` (`shiro.loginUrl`). When the login API responds with 405, the HTTP interceptor emits `session_logout` (https://github.com/apache/zeppelin/blob/f1f671394dc742ec35ede32be039850cdf8a1145/zeppelin-web/src/app/app.js#L170). The login controller listens for `session_logout` and programmatically clicks the login button (`.nav-login-btn`).
   As a result, the login modal pops up immediately when the window opens, which is different from before, and the integration test fails.
   
   It looks like we should change this to issue the API request only when the user is authenticated, or defer it until the moment it is actually needed.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API [zeppelin]

Posted by "tbonelee (via GitHub)" <gi...@apache.org>.
tbonelee commented on code in PR #5099:
URL: https://github.com/apache/zeppelin/pull/5099#discussion_r2413529582


##########
zeppelin-web-angular/src/app/services/configuration.service.ts:
##########
@@ -13,18 +13,33 @@
 import { HttpClient } from '@angular/common/http';
 import { Injectable } from '@angular/core';
 
+import { retry } from 'rxjs/operators';

Review Comment:
   Perhaps we could remove this.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API [zeppelin]

Posted by "tbonelee (via GitHub)" <gi...@apache.org>.
tbonelee commented on PR #5099:
URL: https://github.com/apache/zeppelin/pull/5099#issuecomment-3384711224

   @Reamer Looks reasonable for zeppelin-web too. Are there specific concerns you’re considering, such as maintenance or behavior differences?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API [zeppelin]

Posted by "tbonelee (via GitHub)" <gi...@apache.org>.
tbonelee commented on code in PR #5099:
URL: https://github.com/apache/zeppelin/pull/5099#discussion_r2659567203


##########
zeppelin-web/src/components/note-import/note-import.controller.js:
##########


Review Comment:
   We should set `$scope.maxLimit` value to tell users the max file size.
   
   <img width="557" height="446" alt="Image" src="https://github.com/user-attachments/assets/b44e9401-c5c7-43c1-a8dc-43a3c557938e" />



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API [zeppelin]

Posted by "seung-00 (via GitHub)" <gi...@apache.org>.
seung-00 commented on code in PR #5099:
URL: https://github.com/apache/zeppelin/pull/5099#discussion_r2432115238


##########
zeppelin-server/src/main/java/org/apache/zeppelin/rest/ConfigurationsRestApi.java:
##########
@@ -32,7 +32,7 @@
 import org.apache.zeppelin.service.AuthenticationService;
 
 /** Configurations Rest API Endpoint. */
-@Path("/configurations")

Review Comment:
   @Reamer 
   I also think the path you mentioned looks good. However, since the authentication referenced in `shiro.ini` appears to be set up based on the `/configuration` path, I excluded `/configurations` from the path. The wsMaxMessageSize API is intended to be a non-authenticated API.
   
   Please feel free to let me know if you have any other suggestions. 🤔



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API [zeppelin]

Posted by "tbonelee (via GitHub)" <gi...@apache.org>.
tbonelee commented on code in PR #5099:
URL: https://github.com/apache/zeppelin/pull/5099#discussion_r2429935281


##########
zeppelin-web-angular/src/app/app.module.ts:
##########
@@ -13,7 +13,7 @@
 import { registerLocaleData } from '@angular/common';
 import { HttpClientModule, HTTP_INTERCEPTORS } from '@angular/common/http';
 import en from '@angular/common/locales/en';
-import { NgModule } from '@angular/core';
+import { APP_INITIALIZER, NgModule } from '@angular/core';

Review Comment:
   ```suggestion
   import { NgModule } from '@angular/core';
   ```



##########
zeppelin-web-angular/src/app/app.module.ts:
##########
@@ -26,7 +26,7 @@ import { NzNotificationService } from 'ng-zorro-antd/notification';
 
 import { MESSAGE_INTERCEPTOR, TRASH_FOLDER_ID_TOKEN } from '@zeppelin/interfaces';
 import { loadMonacoBefore } from '@zeppelin/languages';
-import { TicketService } from '@zeppelin/services';
+import { ConfigurationService, TicketService } from '@zeppelin/services';

Review Comment:
   ```suggestion
   import { TicketService } from '@zeppelin/services';
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API [zeppelin]

Posted by "seung-00 (via GitHub)" <gi...@apache.org>.
seung-00 commented on code in PR #5099:
URL: https://github.com/apache/zeppelin/pull/5099#discussion_r2432115238


##########
zeppelin-server/src/main/java/org/apache/zeppelin/rest/ConfigurationsRestApi.java:
##########
@@ -32,7 +32,7 @@
 import org.apache.zeppelin.service.AuthenticationService;
 
 /** Configurations Rest API Endpoint. */
-@Path("/configurations")

Review Comment:
   @Reamer 
   I also think the path you mentioned looks good. However, since the authentication referenced in `shiro.ini` appears to be set up based on the `/configuration` path, I excluded `/configurations` from the path. The wsMaxMessageSize API is intended to be a non-authenticated API.
   Please feel free to let me know if you have any other suggestions.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API [zeppelin]

Posted by "seung-00 (via GitHub)" <gi...@apache.org>.
seung-00 commented on code in PR #5099:
URL: https://github.com/apache/zeppelin/pull/5099#discussion_r2480141374


##########
zeppelin-server/src/main/java/org/apache/zeppelin/rest/ConfigurationsRestApi.java:
##########
@@ -32,7 +32,7 @@
 import org.apache.zeppelin.service.AuthenticationService;
 
 /** Configurations Rest API Endpoint. */
-@Path("/configurations")

Review Comment:
   @Reamer 
   Could you please take a look at this?
   The issue this PR addresses, configuration data being accessible via WebSocket without a permission check because of the `wsMaxMessageSize` field, seems to be a clear problem that needs to be fixed.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API [zeppelin]

Posted by "tbonelee (via GitHub)" <gi...@apache.org>.
tbonelee commented on PR #5099:
URL: https://github.com/apache/zeppelin/pull/5099#issuecomment-3782834206

   Thanks for your work. I merged this PR into the master branch only, since it introduces a breaking change to the authorization behavior.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API [zeppelin]

Posted by "seung-00 (via GitHub)" <gi...@apache.org>.
seung-00 commented on PR #5099:
URL: https://github.com/apache/zeppelin/pull/5099#issuecomment-3394152171

   @Reamer 
   In this case I applied it to both, as you mentioned, to make it easier to remove unreferenced parts later on.
   But I don’t think we need to do that for every task. Covering both sides would be a bit tricky, since the front-end codebases are quite different.
   
   @tbonelee 
   I fixed the issue you mentioned. 
   If the CI failed due to the login modal, I think the CI job might need to be adjusted instead, because the wsMaxMessageSize API doesn’t require authentication.
   However, I found that wsMaxMessageSize was being called and cached initially (same as before), but it’s only needed in specific cases (e.g., exporting/importing note). So I modified it to be called only when required, now the CI error no longer occurs.
   
   BTW, if the user isn’t logged in initially, showing the login modal seems natural.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API [zeppelin]

Posted by "seung-00 (via GitHub)" <gi...@apache.org>.
seung-00 commented on code in PR #5099:
URL: https://github.com/apache/zeppelin/pull/5099#discussion_r2423526845


##########
zeppelin-integration/src/test/java/org/apache/zeppelin/integration/ZeppelinIT.java:
##########
@@ -334,6 +334,7 @@ void testAngularRunParagraph() throws Exception {
 
       // Click on 1 paragraph to trigger z.runParagraph() function
 
+      ZeppelinITUtils.sleep(1000, false);

Review Comment:
   I added a sleep because the test was failing intermittently.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API [zeppelin]

Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on code in PR #5099:
URL: https://github.com/apache/zeppelin/pull/5099#discussion_r2431365878


##########
zeppelin-server/src/main/java/org/apache/zeppelin/rest/ConfigurationsRestApi.java:
##########
@@ -32,7 +32,7 @@
 import org.apache.zeppelin.service.AuthenticationService;
 
 /** Configurations Rest API Endpoint. */
-@Path("/configurations")

Review Comment:
   You have changed the path of the API class here. Probably so that the call is `/api/wsMaxMessageSize`. However, I also think `/api/configurations/wsMaxMessageSize` is very good and would prefer this API endpoint.
   I have changed the path to `/api/wsMaxMessageSize`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API [zeppelin]

Posted by "tbonelee (via GitHub)" <gi...@apache.org>.
tbonelee merged PR #5099:
URL: https://github.com/apache/zeppelin/pull/5099


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API [zeppelin]

Posted by "seung-00 (via GitHub)" <gi...@apache.org>.
seung-00 commented on code in PR #5099:
URL: https://github.com/apache/zeppelin/pull/5099#discussion_r2700817003


##########
zeppelin-web/src/components/note-import/note-import.controller.js:
##########


Review Comment:
   @tbonelee 
   Thanks for the comment. I’ve updated this part.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API [zeppelin]

Posted by "seung-00 (via GitHub)" <gi...@apache.org>.
seung-00 commented on code in PR #5099:
URL: https://github.com/apache/zeppelin/pull/5099#discussion_r2432115238


##########
zeppelin-server/src/main/java/org/apache/zeppelin/rest/ConfigurationsRestApi.java:
##########
@@ -32,7 +32,7 @@
 import org.apache.zeppelin.service.AuthenticationService;
 
 /** Configurations Rest API Endpoint. */
-@Path("/configurations")

Review Comment:
   @Reamer 
   I also think the path you mentioned looks good. However, since the authentication referenced in `shiro.ini` appears to be set up based on the "/configuration" path, I excluded /configurations from the path. The wsMaxMessageSize API is intended to be a non-authenticated API.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API [zeppelin]

Posted by "seung-00 (via GitHub)" <gi...@apache.org>.
seung-00 commented on code in PR #5099:
URL: https://github.com/apache/zeppelin/pull/5099#discussion_r2413801076


##########
zeppelin-web-angular/src/app/services/configuration.service.ts:
##########
@@ -13,18 +13,33 @@
 import { HttpClient } from '@angular/common/http';
 import { Injectable } from '@angular/core';
 
+import { retry } from 'rxjs/operators';

Review Comment:
   @tbonelee 
   Thanks, I've removed.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API [zeppelin]

Posted by "seung-00 (via GitHub)" <gi...@apache.org>.
seung-00 commented on code in PR #5099:
URL: https://github.com/apache/zeppelin/pull/5099#discussion_r2480141374


##########
zeppelin-server/src/main/java/org/apache/zeppelin/rest/ConfigurationsRestApi.java:
##########
@@ -32,7 +32,7 @@
 import org.apache.zeppelin.service.AuthenticationService;
 
 /** Configurations Rest API Endpoint. */
-@Path("/configurations")

Review Comment:
   @Reamer 
   Would you mind taking a look at this?
   The issue this PR addresses, configuration data being accessible via WebSocket without a permission check because of the `wsMaxMessageSize` field, seems to be a clear problem that needs to be fixed.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API [zeppelin]

Posted by "Reamer (via GitHub)" <gi...@apache.org>.
Reamer commented on PR #5099:
URL: https://github.com/apache/zeppelin/pull/5099#issuecomment-3385491543

   > @Reamer Looks reasonable for zeppelin-web too. Are there specific concerns you’re considering, such as maintenance or behavior differences?
   
   Yes both. All frontends should use the same interfaces. This makes it easier to remove things later on.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API [zeppelin]

Posted by "seung-00 (via GitHub)" <gi...@apache.org>.
seung-00 commented on code in PR #5099:
URL: https://github.com/apache/zeppelin/pull/5099#discussion_r2432115238


##########
zeppelin-server/src/main/java/org/apache/zeppelin/rest/ConfigurationsRestApi.java:
##########
@@ -32,7 +32,7 @@
 import org.apache.zeppelin.service.AuthenticationService;
 
 /** Configurations Rest API Endpoint. */
-@Path("/configurations")

Review Comment:
   @Reamer 
   I also think the path you mentioned looks good. However, since the authentication referenced in `shiro.ini` appears to be set up based on the "`/configuration` path, I excluded `/configurations` from the path. The wsMaxMessageSize API is intended to be a non-authenticated API.



##########
zeppelin-server/src/main/java/org/apache/zeppelin/rest/ConfigurationsRestApi.java:
##########
@@ -32,7 +32,7 @@
 import org.apache.zeppelin.service.AuthenticationService;
 
 /** Configurations Rest API Endpoint. */
-@Path("/configurations")

Review Comment:
   @Reamer 
   I also think the path you mentioned looks good. However, since the authentication referenced in `shiro.ini` appears to be set up based on the "``/configuration` path, I excluded `/configurations` from the path. The wsMaxMessageSize API is intended to be a non-authenticated API.



##########
zeppelin-server/src/main/java/org/apache/zeppelin/rest/ConfigurationsRestApi.java:
##########
@@ -32,7 +32,7 @@
 import org.apache.zeppelin.service.AuthenticationService;
 
 /** Configurations Rest API Endpoint. */
-@Path("/configurations")

Review Comment:
   @Reamer 
   I also think the path you mentioned looks good. However, since the authentication referenced in `shiro.ini` appears to be set up based on the `/configuration` path, I excluded `/configurations` from the path. The wsMaxMessageSize API is intended to be a non-authenticated API.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API [zeppelin]

Posted by "seung-00 (via GitHub)" <gi...@apache.org>.
seung-00 commented on PR #5099:
URL: https://github.com/apache/zeppelin/pull/5099#issuecomment-3703756201

   @Reamer @tbonelee 
   As Reamer suggested, I changed the endpoint to a general configuration API for the frontend. As a result, this PR includes a breaking change in shiro.ini.template.
   Thanks for great suggestions and sorry for the one-year delay.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] [ZEPPELIN-6366] Separate WebSocket max message size into a dedicated REST API [zeppelin]

Posted by "seung-00 (via GitHub)" <gi...@apache.org>.
seung-00 commented on PR #5099:
URL: https://github.com/apache/zeppelin/pull/5099#issuecomment-3765364041

   @Reamer 
   Could you review it again?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@zeppelin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org