Skip to content

Conversation

@Leemoonsoo
Copy link
Member

What is this PR for?

Recently #208 and #702 are merged into master branch.
They passed the CI test individually, but failing after both merged.

  • zeppelin-web build error
[ERROR] npm ERR! registry error parsing json
[ERROR] npm http 200 https://registry.npmjs.org/bower/1.7.2
[ERROR] npm ERR! SyntaxError: Unexpected token �
[ERROR] npm ERR! ������Y[o�6��+��6��u��lE���
[ERROR] npm ERR! �{h��C�ˑ�L�=RJ���o�9b�4����W4��["��wn�����E����2C�ϕn���U`���a�
[ERROR] npm ERR! G�p^@��$�e��Ley,��IU�"/K�,qr�[8�F���^���p�������Z�����=x?�}��{W�+���ܳЀ쵱���}

What type of PR is it?

Hot Fix

Todos

  • - Merge R.md and r.md
  • - Fix zeppelin-web build error
  • - Change interpreter listing order

What is the Jira issue?

How should this be tested?

Screenshots (if appropriate)

Questions:

  • Does the licenses files need update? no
  • Is there breaking changes for older versions? no
  • Does this needs documentation? no

@bzz
Copy link
Member

bzz commented Apr 5, 2016

@Leemoonsoo thank you for taking care of CI failure on short notice!

@bzz
Copy link
Member

bzz commented Apr 5, 2016

Looks great to me, let's merge asap as a hotfix

@elbamos
Copy link
Contributor

elbamos commented Apr 5, 2016

Is there a proposal for what Eric and I should do about the name conflict?

On Apr 5, 2016, at 10:21 AM, Alexander [email protected] wrote:

Looks great to me, let's merge asap as a hotfix


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub

@Leemoonsoo
Copy link
Member Author

I'm merging it as a hotfix.

@Leemoonsoo
Copy link
Member Author

Regarding, name conflict, i can come up with some options.

a. Keep the same name 'spark.r' for both SparkRInterpreter and RRepl.
And let user select it in build time using maven profile, -Pr for RRepl, -Psparkr for SparkRInterpreter, or select it in a runtime using zeppelin.interpreters property in conf/zeppelin-site.xml

b. Change SparkRInterpreter name to 'spark.sparkr', similar to PySparkInterpreter uses the name 'spark.pyspark'

c. Change RRepl, and KnitR name from 'spark.r', 'spark.knitr' -> 'r.r', r.knitr'.
And make RRepl and KnitR more like generic R support rather than SparkR support. Similar to what ZEPPELIN-502 trying to do for python

Personally, i'm good with all three options and prefer c) as a long term plan, while my guess is many R users will use r without sparkr integration.

What do you think?

@asfgit asfgit closed this in fb8e77b Apr 5, 2016
@elbamos
Copy link
Contributor

elbamos commented Apr 5, 2016

My preference is b because it imposes the least burden on users.

I think most R users will want spark integration, and I think we disagree about which interpreters provide closer spark integration.

On Apr 5, 2016, at 2:19 PM, Lee moon soo [email protected] wrote:

Regarding, name conflict, i can come up with some options.

a. Keep the same name 'spark.r' for both SparkRInterpreter and RRepl.
And let user select it in build time using maven profile, -Pr for RRepl, -Psparkr for SparkRInterpreter, or select it in a runtime using zeppelin.interpreters property in conf/zeppelin-site.xml

b. Change SparkRInterpreter name to 'spark.sparkr', similar to PySparkInterpreter uses the name 'spark.pyspark'

c. Change RRepl, and KnitR name from 'spark.r', 'spark.knitr' -> 'r.r', r.knitr'.
And make RRepl and KnitR more like generic R support rather than SparkR support. Similar to what ZEPPELIN-502 trying to do for python

Personally, i'm good with all three options and prefer c) as a long term plan, while my guess is many R users will use r without sparkr integration.

What do you think?


You are receiving this because you commented.
Reply to this email directly or view it on GitHub

asfgit pushed a commit that referenced this pull request Apr 6, 2016
### What is this PR for?
Somehow final squashed merge commit for #815 - fb8e77b does not include 2 changes out of 3 changes:
 - eeb411e
 - 9baf57b

This PR re-applies Leemoonsoo work on master and should fix the CI

### What type of PR is it?
Hot Fix

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

Author: Alexander Bezzubov <[email protected]>

Closes #820 from bzz/re-apply-SparkR-implementation-conflict-815 and squashes the following commits:

60267b4 [Alexander Bezzubov] Change interpreter listing order, re-apply eeb411e
7ec1e2c [Alexander Bezzubov] Change node and npm version, re-apply 9baf57b
onkarshedge pushed a commit to onkarshedge/incubator-zeppelin that referenced this pull request May 11, 2016
### What is this PR for?
Recently apache#208 and apache#702 are merged into master branch.
They passed the CI test individually, but failing after both merged.

* zeppelin-web build error
```
[ERROR] npm ERR! registry error parsing json
[ERROR] npm http 200 https://registry.npmjs.org/bower/1.7.2
[ERROR] npm ERR! SyntaxError: Unexpected token �
[ERROR] npm ERR! ������Y[o�6��+��6��u��lE���
[ERROR] npm ERR! �{h��C�ˑ�L�=RJ���o�9b�4����W4��["��wn�����E����2C�ϕn���U`���a�
[ERROR] npm ERR! G�p^��$�e��Ley,��IU�"/K�,qr�[8�F���^���p�������Z�����=x?�}��{W�+���ܳЀ쵱���}
```

* 'SparkRInterpreter.java' and 'RRepl.java' uses the same interpreter name. 'spark.r'.
That conflicts and make https://github.com/apache/incubator-zeppelin/blob/master/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinSparkClusterTest.java#L87 fails.

* R.md and r.md both exists under same directory. That confuses git client.

### What type of PR is it?
Hot Fix

### Todos
* [x] - Merge R.md and r.md
* [x] - Fix zeppelin-web build error
* [x] - Change interpreter listing order

### What is the Jira issue?

### How should this be tested?

### 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: Lee moon soo <[email protected]>

This patch had conflicts when merged, resolved by
Committer: Lee moon soo <[email protected]>

Closes apache#815 from Leemoonsoo/r_hotfix and squashes the following commits:

eeb411e [Lee moon soo] Change interpreter listing order
9baf57b [Lee moon soo] Change node and npm version
6854ac7 [Lee moon soo] R.md -> r.md
onkarshedge pushed a commit to onkarshedge/incubator-zeppelin that referenced this pull request May 11, 2016
### What is this PR for?
Somehow final squashed merge commit for apache#815 - fb8e77b does not include 2 changes out of 3 changes:
 - eeb411e
 - 9baf57b

This PR re-applies Leemoonsoo work on master and should fix the CI

### What type of PR is it?
Hot Fix

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

Author: Alexander Bezzubov <[email protected]>

Closes apache#820 from bzz/re-apply-SparkR-implementation-conflict-815 and squashes the following commits:

60267b4 [Alexander Bezzubov] Change interpreter listing order, re-apply eeb411e
7ec1e2c [Alexander Bezzubov] Change node and npm version, re-apply 9baf57b
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.

3 participants