Skip to content

ZEPPELIN-1185. ZEPPELIN_INTP_JAVA_OPTS should not use ZEPPELIN_JAVA_OPTS#1189

Closed
zjffdu wants to merge 4 commits intoapache:masterfrom
zjffdu:ZEPPELIN-1185
Closed

ZEPPELIN-1185. ZEPPELIN_INTP_JAVA_OPTS should not use ZEPPELIN_JAVA_OPTS#1189
zjffdu wants to merge 4 commits intoapache:masterfrom
zjffdu:ZEPPELIN-1185

Conversation

@zjffdu
Copy link
Contributor

@zjffdu zjffdu commented Jul 15, 2016

What is this PR for?

Don't use ZEPPELIN_JAVA_OPTS as the default value of ZEPPELIN_INTP_JAVA_OPTS

What type of PR is it?

Improvement

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-1185

How should this be tested?

Tested manually. By exporting the following variable, I can debug zeppelin server correctly and remote interpreter process can ran successfully. (Before this PR, the remote interpreter process will fail to launch because it would also listen the same debug port)

export ZEPPELIN_JAVA_OPTS="-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=5005"

@Leemoonsoo
Copy link
Member

Thanks for contribution!

ZEPPELIN_JAVA_OPTS used to set JVM options both ZeppelinServer and InterpreterProcess.
Later ZEPPELIN_INTP_JAVA_OPTS has been introduced. To keep compatibility with previous configuration, ZEPPELIN_INTP_JAVA_OPTS designed to overrides ZEPPELIN_JAVA_OPTS for InterpreterProcess.
So previous version of conf/zeppelin-env.sh can work without modification.
That's why InterpreterProcess looks like implicitly uses ZEPPELIN_JAVA_OPTS.

I don't have any preference of either keeping current behavior or change it.

But if you'd like to change, I think ZEPPELIN_INTP_MEM also need to be changed, to make configuration consistent.

Also, this change makes conf/zeppelin-env.sh incompatible to previous version. So i think it's great idea that guide user how ZEPPELIN_JAVA_OPTS and ZEPPELIN_INTP_JAVA_OPTS are changed through https://github.com/apache/zeppelin/blob/master/docs/install/upgrade.md.

asfgit pushed a commit that referenced this pull request Jul 20, 2016
### What is this PR for?
Adds websocket api for getting note revision.

### What type of PR is it?
Improvement | Feature

### Todos
* [x] - add backend websocket handle
* [x] - add frontend call

### What is the Jira issue?
[#1189](https://issues.apache.org/jira/browse/ZEPPELIN-1189)

### How should this be tested?
green CI (can be tested once frontend implemented)

### Screenshots (if appropriate)

### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? no

Author: Khalid Huseynov <[email protected]>
Author: Khalid Huseynov <[email protected]>
Author: Khalid Huseynov <[email protected]>

Closes #1192 from khalidhuseynov/versioning/get-note-revision-api and squashes the following commits:

f1ab994 [Khalid Huseynov] Merge branch 'master' into versioning/get-note-revision-api
683b481 [Khalid Huseynov] receive Revision object from frontend
aa0a7d6 [Khalid Huseynov] Revert "change NotebookRepo api to get note revision from Revision object to String revId"
ce097ed [Khalid Huseynov] add throws to notebook getRevisionNote
baaa704 [Khalid Huseynov] change NotebookRepo api to get note revision from Revision object to String revId
d9751c3 [Khalid Huseynov] add backend ws api to get note revision
3783fc9 [Khalid Huseynov] receive ws NOTE_REVISION msg
79f8ac9 [Khalid Huseynov] add getNoteRevision to front
@zjffdu
Copy link
Contributor Author

zjffdu commented Jul 26, 2016

@Leemoonsoo I pushed another commit to add migration section in upgrade.md, please help review. Thanks.

``` No newline at end of file
```

# Migration Guide
Copy link
Contributor

@AhyoungRyu AhyoungRyu Jul 26, 2016

Choose a reason for hiding this comment

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

Could you change # -> ## ? Since # is used only for the docs title in many other pages and # won't be rendered under TOC as well.

@zjffdu
Copy link
Contributor Author

zjffdu commented Jul 26, 2016

Thanks @AhyoungRyu , I have updated the doc.

@zjffdu
Copy link
Contributor Author

zjffdu commented Jul 26, 2016

BTW, I will do the windows related change (common.cmd, zeppelin-env.cmd.template) after we make consensus on this approach.

@zjffdu
Copy link
Contributor Author

zjffdu commented Aug 4, 2016

\cc @Leemoonsoo Ready for review.

@zjffdu zjffdu closed this Aug 7, 2016
@zjffdu zjffdu reopened this Aug 7, 2016
@Leemoonsoo
Copy link
Member

Change looks good.
Could you trigger CI once again and see if it becomes green?

@zjffdu zjffdu closed this Aug 8, 2016
@zjffdu zjffdu reopened this Aug 8, 2016
@zjffdu zjffdu closed this Aug 8, 2016
@zjffdu zjffdu reopened this Aug 8, 2016
PhilippGrulich pushed a commit to SWC-SENSE/zeppelin that referenced this pull request Aug 8, 2016
### What is this PR for?
Adds websocket api for getting note revision.

### What type of PR is it?
Improvement | Feature

### Todos
* [x] - add backend websocket handle
* [x] - add frontend call

### What is the Jira issue?
[apache#1189](https://issues.apache.org/jira/browse/ZEPPELIN-1189)

### How should this be tested?
green CI (can be tested once frontend implemented)

### Screenshots (if appropriate)

### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? no

Author: Khalid Huseynov <[email protected]>
Author: Khalid Huseynov <[email protected]>
Author: Khalid Huseynov <[email protected]>

Closes apache#1192 from khalidhuseynov/versioning/get-note-revision-api and squashes the following commits:

f1ab994 [Khalid Huseynov] Merge branch 'master' into versioning/get-note-revision-api
683b481 [Khalid Huseynov] receive Revision object from frontend
aa0a7d6 [Khalid Huseynov] Revert "change NotebookRepo api to get note revision from Revision object to String revId"
ce097ed [Khalid Huseynov] add throws to notebook getRevisionNote
baaa704 [Khalid Huseynov] change NotebookRepo api to get note revision from Revision object to String revId
d9751c3 [Khalid Huseynov] add backend ws api to get note revision
3783fc9 [Khalid Huseynov] receive ws NOTE_REVISION msg
79f8ac9 [Khalid Huseynov] add getNoteRevision to front
@zjffdu
Copy link
Contributor Author

zjffdu commented Aug 17, 2016

ping @Leemoonsoo in case you miss it.

@bzz
Copy link
Member

bzz commented Aug 17, 2016

I guess @khalidhuseynov did not mean to refer this PR, but just used short-cut to name the JIRA issue.

@Leemoonsoo
Copy link
Member

Thanks @zjffdu.
LGTM and merge if there's no more discussion.

@prabhjyotsingh
Copy link
Contributor

I'll merge this in sometime soon.

@asfgit asfgit closed this in 93e3762 Sep 1, 2016
asfgit pushed a commit that referenced this pull request Sep 1, 2016
### What is this PR for?

Don't use ZEPPELIN_JAVA_OPTS as the default value of ZEPPELIN_INTP_JAVA_OPTS

### What type of PR is it?
Improvement

### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-1185

### How should this be tested?
Tested manually. By exporting the following variable, I can debug zeppelin server correctly and remote interpreter process can ran successfully. (Before this PR, the remote  interpreter process will fail to launch because it would also listen the same debug port)
```
export ZEPPELIN_JAVA_OPTS="-agentlib:jdwp=transport=dt_socket,server=y,suspend=y,address=5005"
```

Author: Jeff Zhang <[email protected]>

Closes #1189 from zjffdu/ZEPPELIN-1185 and squashes the following commits:

9e48ad7 [Jeff Zhang] change for windows
3ff5561 [Jeff Zhang] update doc format
e82d889 [Jeff Zhang] add migration doc
ef5a360 [Jeff Zhang] ZEPPELIN-1185. ZEPPELIN_INTP_JAVA_OPTS should not use ZEPPELIN_JAVA_OPTS as default value

(cherry picked from commit 93e3762)
Signed-off-by: Prabhjyot Singh <[email protected]>
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.

5 participants