Skip to content

ZEPPELIN-172 Websocket connection without separate port #170

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.

Sign up for GitHub

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

Jump to bottom
Closed
Leemoonsoo wants to merge 9 commits into from
Closed

ZEPPELIN-172 Websocket connection without separate port #170

Leemoonsoo wants to merge 9 commits into from

Conversation

@Leemoonsoo
Copy link
Member

@Leemoonsoo Leemoonsoo commented Jul 28, 2015

@corneadoug
Copy link
Contributor

corneadoug commented Jul 28, 2015

Didn't try it yet,
But front part is well changed and Overall looks like a nice job.

@Leemoonsoo
Copy link
Member Author

Leemoonsoo commented Jul 28, 2015

Ready to merge

@corneadoug
Copy link
Contributor

corneadoug commented Jul 29, 2015

There is a couple of other things related to Websocket, do you think this could be handled in this PR too? Security: https://issues.apache.org/jira/browse/ZEPPELIN-173
PingPong: https://issues.apache.org/jira/browse/ZEPPELIN-113 (Last comments)

sjoerdmulder
sjoerdmulder reviewed Jul 30, 2015
View reviewed changes
zeppelin-server/src/main/java/org/apache/zeppelin/server/AppScriptServlet.java Outdated
Copy link
Contributor

@sjoerdmulder sjoerdmulder Jul 30, 2015

Choose a reason for hiding this comment

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

This will set an incorrect internal local port of the zeppelin instance (e.g. 8080) where it is possible that there is a reverse proxy (nginx with HTTPS for example) in front of zeppelin.

Actually I think this whole Servlet is no longer needed since the websocket port is now the http port, so the browser can simply assume location.port

Copy link
Contributor

@corneadoug corneadoug Jul 30, 2015

Choose a reason for hiding this comment

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

Yeah, you are right replacing getPort is not needed anymore

@sjoerdmulder
Copy link
Contributor

sjoerdmulder commented Jul 30, 2015

I have created an pull request cleaning it up some more: Leemoonsoo#2

@Leemoonsoo
Merge pull request #2 from sjoerdmulder/websocket
3cf839d 
Cleanup of Javascript logic and Server code detecting the correct port
@Leemoonsoo
Copy link
Member Author

Leemoonsoo commented Aug 2, 2015

Thanks @sjoerdmulder

@Leemoonsoo
Copy link
Member Author

Leemoonsoo commented Aug 2, 2015

Ready to merge

@Leemoonsoo
Copy link
Member Author

Leemoonsoo commented Aug 4, 2015

merging if there're no more discussions.

@corneadoug
Copy link
Contributor

corneadoug commented Aug 4, 2015

Go for it :) +1

@asfgit asfgit closed this in 3a42a28 Aug 5, 2015
Leemoonsoo added a commit to Leemoonsoo/zeppelin that referenced this pull request Sep 17, 2015
@Leemoonsoo
ZEPPELIN-172 Websocket connection without separate port
b7a5140 
This PR fixes https://issues.apache.org/jira/browse/ZEPPELIN-172

Author: Lee moon soo <moon@apache.org>
Author: Sjoerd Mulder <sjoerd@frontendless.nl>
Author: Lee moon soo <leemoonsoo@gmail.com>

Closes apache#170 from Leemoonsoo/websocket and squashes the following commits:

11a302a [Lee moon soo] Check text in more safe way
3cf839d [Lee moon soo] Merge pull request #2 from sjoerdmulder/websocket
7f8bc47 [Sjoerd Mulder] Cleanup of Javascript logic and Server code detecting the correct port
412927f [Lee moon soo] Handle large message
f56e417 [Lee moon soo] Add license header
806db9b [Lee moon soo] Remove websocket addr/port configuration
6180ed3 [Lee moon soo] Update README
85d14a0 [Lee moon soo] Create notebookserver instance manually
a7b82aa [Lee moon soo] Initial implementation of Websocket inside of Jetty server

(cherry picked from commit 3a42a28)
Signed-off-by: Lee moon soo <moon@apache.org>
mapr-devops pushed a commit to mapr/zeppelin that referenced this pull request May 8, 2025
@idzikovskyi
Enable PAM auth by default and allow authenticated users to configure…
58e7ecb 
… interpreters and add admin users

(cherry picked from commit e2cab20)

ZEP-225 Added admin user (apache#169)

* Added admin user

* changed shiro.ini to shiro.ini.template

* Configure admin role through configure.sh (apache#170)

Co-authored-by: Ivan Dzikovskyi <ivan.dzikovskyi@hpe.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Leemoonsoo @corneadoug @sjoerdmulder