Skip to content

Conversation

@mwkang
Copy link
Contributor

@mwkang mwkang commented Mar 30, 2016

What is this PR for?

Automatically adds %pyspark in the code area after paragraph created, if previous paragraph's typing is %pyspark.

What type of PR is it?

New Feature

What is the Jira issue?

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

How should this be tested?

unit test
Run-time checking

Screenshots (if appropriate)

  • default interpreter
    zeppelin-707
  • while run paragraph, go to line end
    zeppelin-707-2

@mwkang mwkang changed the title [ZEPPELIN-707]Automatically adds %.* of previous paragraph [ZEPPELIN-707]Automatically adds %.* of previous paragraph's typing Mar 30, 2016
@Leemoonsoo
Copy link
Member

Zeppelin automatically adds new paragraph when last paragraph runs.
Isn't it make sense to add %.* to this auto added paragraph?

@mwkang
Copy link
Contributor Author

mwkang commented Mar 31, 2016

I agree with you. This commit is too hasty.
I'm so sorry. I'm looking in that.

@mwkang
Copy link
Contributor Author

mwkang commented Mar 31, 2016

@Leemoonsoo
I changed some code. Could you mind if I request to review this?

@corneadoug
Copy link
Contributor

Isn't it possible not to have any front-end code for that?
Why not just include the %.. in the empty note in the backend?

@mwkang
Copy link
Contributor Author

mwkang commented Mar 31, 2016

The purpose of this front-end code is to track the recent usage of the interpteter(by the users).
Okay. include the %... in the empty note.

@felixcheung
Copy link
Member

Does this work when the user has not included %interpreter in the previous paragraph? (ie. using the default interpreter)

@mwkang
Copy link
Contributor Author

mwkang commented Apr 2, 2016

@felixcheung
Yes, it works.
If user doesn't any %interpreter in previous paragraphs, %..(default interpreter) will be added.
If user add %interpreter in previous paragraphs, %interpreter will be added.

e.g.)
paragraph-1 (no interpreters) -> paragraph-2 (no interpreters) -> paragraph-3 (%.. will be added.)
paragraph-1 (%sh) -> paragraph-2 (no interpreters) -> paragraph-3 (%sh will be added.)

@mwkang
Copy link
Contributor Author

mwkang commented Apr 2, 2016

Is there anything wrong?

@bzz
Copy link
Member

bzz commented Apr 4, 2016

@mwkang Great job! But as you can see - CI is failing now, you probably need to check in zeppelin-webapp what does ./grunt jshint complain about.

@corneadoug
Copy link
Contributor

For me, same comment as before.
There is no need for any front-end code.

@mwkang
Copy link
Contributor Author

mwkang commented Apr 4, 2016

@bzz Thank you for your review.
@corneadoug Okay, I will change the code does not use a front-end code.

@mwkang
Copy link
Contributor Author

mwkang commented Apr 4, 2016

@corneadoug I have a question. If User doesn't run paragraph, Always add %... Because I couldn't know what is recent usage of the interpreter(by the users). Is it okay?

@corneadoug
Copy link
Contributor

If we create a new empty paragraph, we should add the %.. (default, or previous).
So this should be working for: running the last paragraph of the note + adding a new paragraph

@mwkang
Copy link
Contributor Author

mwkang commented Apr 4, 2016

I change my code it is use only backend. I force push my remote branch. So remove previous commit.

@mwkang
Copy link
Contributor Author

mwkang commented Apr 11, 2016

@corneadoug
Could you review this?

@corneadoug
Copy link
Contributor

@mwkang

I found 2 problems:

  1. Content of the first line is copied instead of interpreter sign only, for example:
%md hello

Will copy %md hello to the new paragraph, instead of %md

  1. Default interpreter is not copied
    If you type code using the default interpreter, it will add the previous paragraph interpreter sign instead.
    So If you have:

Paragraph 1

%md
hello

Paragraph 2

println("Hello")

The new paragraph will have %md

@corneadoug
Copy link
Contributor

@mwkang Do you have time to fix this PR?

@mwkang
Copy link
Contributor Author

mwkang commented Apr 29, 2016

@corneadoug
Yes, Sorry for delay.

@mwkang
Copy link
Contributor Author

mwkang commented May 1, 2016

@corneadoug
I fixed that problems.
Could you mind if I request review to you?

@mwkang
Copy link
Contributor Author

mwkang commented May 9, 2016

I'm so sorry. CI was failed.
I'll try to fix it.
(And.. I'm sorry for I has been dragging on for a month.)

@corneadoug
Copy link
Contributor

@mwkang no problem
It seems some tests and selectors are broken with the new changes.
I will try the branch again and post if I find what is breaking at the same time

@mwkang
Copy link
Contributor Author

mwkang commented May 18, 2016

@corneadoug @Leemoonsoo Yeah! I passed all test.

@corneadoug
Copy link
Contributor

@mwkang
Sorry for late answer, I tested again and have 2 comments:

  • When you create a new notebook, the interpreter is listed as: %... Any way to get the default interpreter from back-end and insert it instead?
  • After running a query, the cursor is placed on the next paragraph where the interpreter name was injected, however the cursor is placed at the beginning of the line (so you can't start typing)

@mwkang
Copy link
Contributor Author

mwkang commented May 24, 2016

@corneadoug
Thank you for your comment.
I am sorry I don't understand clearly.
Could you explain more?
What is the meaning get the default interpreter from back-end and insert it instead?
And I think %.. is default interpreter, isn't it?

(I am so sorry I think I am a stupid. 😢)

@corneadoug
Copy link
Contributor

I don't remember if there was a way to change the default interpreter.
But at least without config, it uses '%spark' by default, so
sc.version == %spark sc.version

In our case, if we try running

%..
sc.version

You will get a: %.. interpreter not found

@mwkang
Copy link
Contributor Author

mwkang commented May 24, 2016

@corneadoug
Oh... I don't consider about that.
Thanks for mention that,
I will fix my code.

@mwkang
Copy link
Contributor Author

mwkang commented May 30, 2016

  • When you create a new notebook, the interpreter is listed as: %... Any way to get the default interpreter from back-end and insert it instead?
  • After running a query, the cursor is placed on the next paragraph where the interpreter name was injected, however the cursor is placed at the beginning of the line (so you can't start typing)
  • Fix CI ([ERROR] /home/travis/build/apache/incubator-zeppelin/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java:[718,5] cannot find symbol
    symbol: class PutMethod
    location: class org.apache.zeppelin.rest.ZeppelinRestApiTest)
[ERROR] /home/travis/build/apache/incubator-zeppelin/zeppelin-server/src/test/java/org/apache/zeppelin/rest/ZeppelinRestApiTest.java:[718,5] cannot find symbol
  symbol:   class PutMethod
  location: class org.apache.zeppelin.rest.ZeppelinRestApiTest

@mwkang
Copy link
Contributor Author

mwkang commented May 30, 2016

@corneadoug
Hello, I have a question.
Only after running a query, the cursor go to line end in new paragraph.
If It is added manually, the cursor doesn't go to line end.
Added manually meaning is using add button in paragraph menu or click add button between two paragraphs.
Is it Okay or Should I make the cursor go to line end without exception?

@corneadoug
Copy link
Contributor

Since the cursor is moving in the text editor of the new paragraph, it should also go at the end of the line

@mwkang
Copy link
Contributor Author

mwkang commented Jun 20, 2016

@jongyoul I rebase this PR onto master. I'll create new PR about It ignore wrong interpreter name

@jongyoul
Copy link
Member

@mwkang Could you please re-trigger test once again?

@mwkang mwkang closed this Jun 20, 2016
@mwkang mwkang reopened this Jun 20, 2016
@mwkang
Copy link
Contributor Author

mwkang commented Jun 20, 2016

I reopened this PR. But I think I have to fix some test case.

@mwkang
Copy link
Contributor Author

mwkang commented Jun 20, 2016

Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 104.235 sec - in org.apache.zeppelin.integration.ZeppelinIT

Results :

Failed tests: 
  ParagraphActionsIT.testTitleButton:343 After Show Title : The title field contains
Expected: "Untitled"
     but: was ""

Tests in error: 
  ParagraphActionsIT.testRemoveButton:143->AbstractZeppelinIT.waitForParagraph:84->AbstractZeppelinIT.pollingWait:110 쨩 Timeout
  ParagraphActionsIT.testMoveUpAndDown:180->AbstractZeppelinIT.waitForParagraph:84->AbstractZeppelinIT.pollingWait:110 쨩 Timeout
  ParagraphActionsIT.testCreateNewButton:75->AbstractZeppelinIT.waitForParagraph:84->AbstractZeppelinIT.pollingWait:110 쨩 Timeout

@jongyoul
Copy link
Member

Yes, those test are not relevant to you. LGTM. I'll merge it into master.

@asfgit asfgit closed this in dec31d6 Jun 20, 2016
@jongyoul
Copy link
Member

Thanks for the contribution. I'm looking forward to see your next PR.

@mwkang
Copy link
Contributor Author

mwkang commented Jun 20, 2016

Wow thanks for your patience. This PR is a long period and lots of comments. I'll keep on trying my best.

asfgit pushed a commit that referenced this pull request Jun 23, 2016
### What is this PR for?
This PR is to fix bug from #806.
The default interpreter name that automatically adds should be interpreter group name.

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

### Screenshots (if appropriate)
  - before
![b](https://cloud.githubusercontent.com/assets/3348133/16221424/99cab6b6-37cd-11e6-8074-93fd9db53b45.gif)

  - after
![a](https://cloud.githubusercontent.com/assets/3348133/16221623/d2c36e94-37ce-11e6-957f-fea2e67dd55d.gif)

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

Author: astroshim <hsshim@nflabs.com>

Closes #1058 from astroshim/bugfix/ZEPPELIN-707 and squashes the following commits:

8e00b49 [astroshim] default interpreter name should be group name.
asfgit pushed a commit that referenced this pull request Sep 2, 2016
…rpreter name

### What is this PR for?
Ignore implicit interpreter when user enter wrong interpreter name
linked to #806 (comment)

This PR is related to #1113
ZEPPELIN-1069 branch was force push, so couldn't reopen that PR.

### What type of PR is it?
Improvement

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

### How should this be tested?
Unit test
Run-time checking

### Screenshots (if appropriate)
![zeppelin-1069-gif](https://cloud.githubusercontent.com/assets/10624086/17268431/d6d9370c-5664-11e6-9274-d0244d60a8c9.gif)

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

Author: Minwoo Kang <minwoo.kang@outlook.com>

Closes #1248 from mwkang/ZEPPELIN-1069 and squashes the following commits:

3976a96 [Minwoo Kang] Remove wild card import
fa7a194 [Minwoo Kang] Fix CI failed
f0eedeb [Minwoo Kang] Remove unnecessary method.
6e91dfe [Minwoo Kang] Remove unnecessary method.
dd60d72 [Minwoo Kang] Fixed CI
046f5f4 [Minwoo Kang] Modify static import position.
6082e28 [Minwoo Kang] Modify that StringUtils is static import
02a1c2a [Minwoo Kang] Refactor method name
4912c5c [Minwoo Kang] Add test cases
b6520be [Minwoo Kang] Add function that is replName is binding.
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.

6 participants