You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by astroshim <gi...@git.apache.org> on 2016/06/29 00:32:26 UTC
[GitHub] zeppelin pull request #1104: [ZEPPELIN-1078] Does not show the text result i...
GitHub user astroshim opened a pull request:
https://github.com/apache/zeppelin/pull/1104
[ZEPPELIN-1078] Does not show the text result in the paragraph sometimes.
### What is this PR for?
This PR fixes the problem that does not show the text result in the paragraph sometimes.
### What type of PR is it?
Bug Fix
### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-1078
### How should this be tested?
Try run python code constantly like screenshot.
### Screenshots (if appropriate)
- before
### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? no
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/astroshim/zeppelin ZEPPELIN-1078
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/1104.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1104
----
commit 441357c042ed557c63f18bb15b2833293431bd06
Author: astroshim <hs...@nflabs.com>
Date: 2016-06-29T00:28:41Z
remove resultRefreshed value.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1104: [ZEPPELIN-1078] Does not show the text result in the p...
Posted by corneadoug <gi...@git.apache.org>.
Github user corneadoug commented on the issue:
https://github.com/apache/zeppelin/pull/1104
@astroshim Yes, its the best way for now
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin pull request #1104: [ZEPPELIN-1078] Does not show the text result i...
Posted by astroshim <gi...@git.apache.org>.
Github user astroshim closed the pull request at:
https://github.com/apache/zeppelin/pull/1104
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1104: [ZEPPELIN-1078] Does not show the text result in the p...
Posted by bzz <gi...@git.apache.org>.
Github user bzz commented on the issue:
https://github.com/apache/zeppelin/pull/1104
Thank you for great feedback @corneadoug and prompt fixes @astroshim
Looks great to me!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin pull request #1104: [ZEPPELIN-1078] Does not show the text result i...
Posted by astroshim <gi...@git.apache.org>.
GitHub user astroshim reopened a pull request:
https://github.com/apache/zeppelin/pull/1104
[ZEPPELIN-1078] Does not show the text result in the paragraph sometimes.
### What is this PR for?
This PR fixes the problem that does not show the text result in the paragraph sometimes.
### What type of PR is it?
Bug Fix
### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-1078
### How should this be tested?
Try run python code constantly like screenshot.
### Screenshots (if appropriate)
- before
### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? no
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/astroshim/zeppelin ZEPPELIN-1078
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/1104.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1104
----
commit 441357c042ed557c63f18bb15b2833293431bd06
Author: astroshim <hs...@nflabs.com>
Date: 2016-06-29T00:28:41Z
remove resultRefreshed value.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1104: [ZEPPELIN-1078] Does not show the text result in the p...
Posted by corneadoug <gi...@git.apache.org>.
Github user corneadoug commented on the issue:
https://github.com/apache/zeppelin/pull/1104
LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1104: [ZEPPELIN-1078] Does not show the text result in the p...
Posted by astroshim <gi...@git.apache.org>.
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1104
It seems that the CI fail is related to https://github.com/apache/zeppelin/pull/1084.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1104: [ZEPPELIN-1078] Does not show the text result in the p...
Posted by astroshim <gi...@git.apache.org>.
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1104
@corneadoug I rebased. thanks.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1104: [ZEPPELIN-1078] Does not show the text result in the p...
Posted by astroshim <gi...@git.apache.org>.
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1104
re-trigger CI
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1104: [ZEPPELIN-1078] Does not show the text result in the p...
Posted by astroshim <gi...@git.apache.org>.
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1104
re-trigger CI
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin pull request #1104: [ZEPPELIN-1078] Does not show the text result i...
Posted by astroshim <gi...@git.apache.org>.
Github user astroshim closed the pull request at:
https://github.com/apache/zeppelin/pull/1104
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1104: [ZEPPELIN-1078] Does not show the text result in the p...
Posted by astroshim <gi...@git.apache.org>.
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1104
re-trigger CI
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1104: [ZEPPELIN-1078] Does not show the text result in the p...
Posted by astroshim <gi...@git.apache.org>.
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1104
@bzz Thank you for reviewing.
I think ```resultRefreshed``` condition causes bug and I don't know why it needs.
@corneadoug @felizbear please review.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1104: [ZEPPELIN-1078] Does not show the text result in the p...
Posted by corneadoug <gi...@git.apache.org>.
Github user corneadoug commented on the issue:
https://github.com/apache/zeppelin/pull/1104
@astroshim Can you rebase with master?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin pull request #1104: [ZEPPELIN-1078] Does not show the text result i...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/zeppelin/pull/1104
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1104: [ZEPPELIN-1078] Does not show the text result in the p...
Posted by bzz <gi...@git.apache.org>.
Github user bzz commented on the issue:
https://github.com/apache/zeppelin/pull/1104
So you think `resultRefreshed ` condition is not useful check any more?
\cc @felizbear @corneadoug for review
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin pull request #1104: [ZEPPELIN-1078] Does not show the text result i...
Posted by astroshim <gi...@git.apache.org>.
GitHub user astroshim reopened a pull request:
https://github.com/apache/zeppelin/pull/1104
[ZEPPELIN-1078] Does not show the text result in the paragraph sometimes.
### What is this PR for?
This PR fixes the problem that does not show the text result in the paragraph sometimes.
### What type of PR is it?
Bug Fix
### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-1078
### How should this be tested?
Try run python code constantly like screenshot.
### Screenshots (if appropriate)
- before
### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? no
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/astroshim/zeppelin ZEPPELIN-1078
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/1104.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1104
----
commit 441357c042ed557c63f18bb15b2833293431bd06
Author: astroshim <hs...@nflabs.com>
Date: 2016-06-29T00:28:41Z
remove resultRefreshed value.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1104: [ZEPPELIN-1078] Does not show the text result in the p...
Posted by corneadoug <gi...@git.apache.org>.
Github user corneadoug commented on the issue:
https://github.com/apache/zeppelin/pull/1104
Merging if there is no more discussions
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin pull request #1104: [ZEPPELIN-1078] Does not show the text result i...
Posted by astroshim <gi...@git.apache.org>.
Github user astroshim closed the pull request at:
https://github.com/apache/zeppelin/pull/1104
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1104: [ZEPPELIN-1078] Does not show the text result in the p...
Posted by corneadoug <gi...@git.apache.org>.
Github user corneadoug commented on the issue:
https://github.com/apache/zeppelin/pull/1104
@bzz @astroshim
Took me quite some time to dig into this.
Removing `resultRefreshed` would have negative performances effect, its original goal is to not re-render things if there was no change. however its condition isn't that great and a better one would be: `!angular.equals(data.paragraph.result, $scope.paragraph.result);`
A quick fix is indeed to remove `resultRefreshed`, but from this condition only `else if (newType === 'TEXT' && resultRefreshed) {`
The overall issue is that the implementation of the streaming feature in interpreters for the TEXT result type is not consistent.
And since the issue is complex and need of lot of explanations, I will create a separate issue.
We can keep this PR as a quick fix
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1104: [ZEPPELIN-1078] Does not show the text result in the p...
Posted by astroshim <gi...@git.apache.org>.
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1104
@corneadoug Thank you for making clear. I fixed the code as you advised.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin pull request #1104: [ZEPPELIN-1078] Does not show the text result i...
Posted by astroshim <gi...@git.apache.org>.
GitHub user astroshim reopened a pull request:
https://github.com/apache/zeppelin/pull/1104
[ZEPPELIN-1078] Does not show the text result in the paragraph sometimes.
### What is this PR for?
This PR fixes the problem that does not show the text result in the paragraph sometimes.
### What type of PR is it?
Bug Fix
### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-1078
### How should this be tested?
Try run python code constantly like screenshot.
### Screenshots (if appropriate)
- before
### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? no
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/astroshim/zeppelin ZEPPELIN-1078
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/1104.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1104
----
commit 441357c042ed557c63f18bb15b2833293431bd06
Author: astroshim <hs...@nflabs.com>
Date: 2016-06-29T00:28:41Z
remove resultRefreshed value.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin pull request #1104: [ZEPPELIN-1078] Does not show the text result i...
Posted by astroshim <gi...@git.apache.org>.
Github user astroshim closed the pull request at:
https://github.com/apache/zeppelin/pull/1104
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1104: [ZEPPELIN-1078] Does not show the text result in the p...
Posted by astroshim <gi...@git.apache.org>.
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1104
@corneadoug
I thought it might be impacted but I didn't find a problem so I was waiting these great review!
The implementation of the streaming feature in interpreters for the TEXT result type is already a problem because the paragraph results are cleared before rendering result in the ```$scope.renderText = function() { ``` function.
Then in this PR, can I just remove ```resultRefreshed``` from ```else if (newType === 'TEXT' && resultRefreshed) {```?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin pull request #1104: [ZEPPELIN-1078] Does not show the text result i...
Posted by astroshim <gi...@git.apache.org>.
GitHub user astroshim reopened a pull request:
https://github.com/apache/zeppelin/pull/1104
[ZEPPELIN-1078] Does not show the text result in the paragraph sometimes.
### What is this PR for?
This PR fixes the problem that does not show the text result in the paragraph sometimes.
### What type of PR is it?
Bug Fix
### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-1078
### How should this be tested?
Try run python code constantly like screenshot.
### Screenshots (if appropriate)
- before
### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? no
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/astroshim/zeppelin ZEPPELIN-1078
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/1104.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1104
----
commit 441357c042ed557c63f18bb15b2833293431bd06
Author: astroshim <hs...@nflabs.com>
Date: 2016-06-29T00:28:41Z
remove resultRefreshed value.
commit edf239774035a246c461997fde3e37ed3c07148a
Author: astroshim <hs...@nflabs.com>
Date: 2016-07-02T15:15:33Z
Merge branch 'master' into ZEPPELIN-1078
revert
commit ec44166ab416d85e9ab5ee614ed816c9d099d44a
Author: astroshim <hs...@nflabs.com>
Date: 2016-07-02T15:22:17Z
remove resultRefreshed from TEXT type only
commit 189c5eb432fd0997cf7fd2e53b4fc1fa66b7291a
Author: astroshim <hs...@nflabs.com>
Date: 2016-07-02T15:23:41Z
remove resultRefreshed from TEXT type only
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] zeppelin issue #1104: [ZEPPELIN-1078] Does not show the text result in the p...
Posted by corneadoug <gi...@git.apache.org>.
Github user corneadoug commented on the issue:
https://github.com/apache/zeppelin/pull/1104
I'm currently writing a giant review as I'm testing things
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---