Skip to content

Conversation

@AhyoungRyu
Copy link
Contributor

@AhyoungRyu AhyoungRyu commented Jan 2, 2017

What is this PR for?

Shell interpreter streaming output had been available by #683, but currently it's broken after #1087 merged. This patch is for putting it back.

What type of PR is it?

Bug Fix

TODO

  • Fix test

What is the Jira issue?

ZEPPELIN-1880

How should this be tested?

%sh

date && sleep 3 && date

the each timestamp must be printed as streaming output

Screenshots (if appropriate)

  • before
    shellintpresultbefore

  • after
    shellintpresult

Questions:

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

@astroshim
Copy link
Contributor

Hi.
In my case it seems doesn`t work properly.. What am i missing?
test1

@Leemoonsoo
Copy link
Member

Tested but i also see the same result @astroshim has.

@AhyoungRyu
Copy link
Contributor Author

Seems the streaming output works only once right after I start Zeppelin (or restart shell intp). Let me dig into more. Thanks for the review @astroshim @Leemoonsoo!

@AhyoungRyu
Copy link
Contributor Author

AhyoungRyu commented Jan 3, 2017

@astroshim @Leemoonsoo I guess the issue that I told in the above is not a matter of Shell intp itself. I found wired behaviour of current streaming output feature. This occurs in master branch.

If I use only Spark, it's streaming output works properly like below.

  • Run only Spark (tried 3 times in a row)
%spark

(1 to 5).foreach{ i=>
    Thread.sleep(500)
    println("Hello " + i)
}

runonlyspark

But if I use Spark -> Shell -> Spark intp, then Spark's streaming output also doesn't work.

  • Run Spark -> Shell -> Spark
    runsparkandsh

@Leemoonsoo Are there any suspicious points in this problem? I tested also in 0.6.2, but the above issue is not in 0.6.2.

@soralee
Copy link
Contributor

soralee commented Jan 4, 2017

I tested and I also faced the problem in this branch. (It is same problem with above @AhyoungRyu's comment.)

At first time, It was worked very well.
z1833_c

But when I rerun same paragraph, it was not worked.
z1833_c_1

@AhyoungRyu
Copy link
Contributor Author

@soralee Thanks for testing it out. Right I think the rerunning issue is due to that I mentioned in this comment.

@astroshim @Leemoonsoo BTW just to be making sure, did you build this branch including shell interpreter like mvn clean package -DskipTests -pl 'shell, zeppelin-interpreter, zeppelin-server, zeppelin-zengine' to test this patch? After build and restart Zeppelin, the result should be same with @soralee and me.

asfgit pushed a commit that referenced this pull request Jan 23, 2017
### What is this PR for?
If you run the following code, then streaming output doesn't work properly from the second run.
```
%spark.pyspark
import time
print("1")
time.sleep(2)
print("2")
time.sleep(2)
print("3")
time.sleep(2)
print("4")
```
This problem comes from the order of `paragraph update` event timing and `paragraph update-append` event timing is incorrect.
and This PR will fix also #1833 too.

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

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

### How should this be tested?
- run several times pyspark interpreter with above code.

### Screenshots (if appropriate)
- before
![2017-01-21 00_55_25](https://cloud.githubusercontent.com/assets/3348133/22173437/bfa48e64-df77-11e6-9625-ab44dedee395.gif)

- after
![2017-01-21 00_59_12](https://cloud.githubusercontent.com/assets/3348133/22173438/c21820ac-df77-11e6-87dc-07970fca13ca.gif)

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

Author: astroshim <[email protected]>

Closes #1927 from astroshim/ZEPPELIN-1994 and squashes the following commits:

c7baa59 [astroshim] fix streaming output problem
asfgit pushed a commit that referenced this pull request Jan 23, 2017
### What is this PR for?
If you run the following code, then streaming output doesn't work properly from the second run.
```
%spark.pyspark
import time
print("1")
time.sleep(2)
print("2")
time.sleep(2)
print("3")
time.sleep(2)
print("4")
```
This problem comes from the order of `paragraph update` event timing and `paragraph update-append` event timing is incorrect.
and This PR will fix also #1833 too.

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

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

### How should this be tested?
- run several times pyspark interpreter with above code.

### Screenshots (if appropriate)
- before
![2017-01-21 00_55_25](https://cloud.githubusercontent.com/assets/3348133/22173437/bfa48e64-df77-11e6-9625-ab44dedee395.gif)

- after
![2017-01-21 00_59_12](https://cloud.githubusercontent.com/assets/3348133/22173438/c21820ac-df77-11e6-87dc-07970fca13ca.gif)

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

Author: astroshim <[email protected]>

Closes #1927 from astroshim/ZEPPELIN-1994 and squashes the following commits:

c7baa59 [astroshim] fix streaming output problem

(cherry picked from commit 1a1fbc4)
Signed-off-by: ahyoungryu <[email protected]>
@AhyoungRyu
Copy link
Contributor Author

I rebased from master after #1922 and #1927 merged. Both are fixing the problem that I mentioned in this comment. Here is the screenshot.
record

@astroshim Thanks again for the fix!!

@AhyoungRyu
Copy link
Contributor Author

@astroshim @Leemoonsoo @soralee If you don't mind, can someone help review this again? :)

@astroshim
Copy link
Contributor

LGTM it's working well.

@soralee
Copy link
Contributor

soralee commented Jan 23, 2017

Tested and it works very well!

@Leemoonsoo
Copy link
Member

Tested. LGTM

@AhyoungRyu
Copy link
Contributor Author

Thanks all you guys for testing it out!
Will merge if there are no more comments.

@asfgit asfgit closed this in 9b4a1bf Jan 24, 2017
asfgit pushed a commit that referenced this pull request Jan 24, 2017
### What is this PR for?
Shell interpreter streaming output had been available by #683, but currently it's broken after #1087 merged. This patch is for putting it back.

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

### TODO
- [x] Fix test

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

### How should this be tested?
```
%sh

date && sleep 3 && date
```

the each timestamp must be printed as streaming output

### Screenshots (if appropriate)
 - before
![shellintpresultbefore](https://cloud.githubusercontent.com/assets/10060731/21585515/60c35a04-d105-11e6-8e68-853ee784e89d.gif)

 - after
![shellintpresult](https://cloud.githubusercontent.com/assets/10060731/21585516/62142ac8-d105-11e6-8628-1d6eec35daae.gif)

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

Author: AhyoungRyu <[email protected]>

Closes #1833 from AhyoungRyu/ZEPPELIN-1880 and squashes the following commits:

8fe33c4 [AhyoungRyu] Fix invalid test cases
e2fd4bf [AhyoungRyu] Add test for shell inpt timeout property
34d3021 [AhyoungRyu] Fix shell intp streaming output result

(cherry picked from commit 9b4a1bf)
Signed-off-by: ahyoungryu <[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.

4 participants