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/09/18 13:38:48 UTC
[GitHub] zeppelin pull request #1433: [ZEPPELIN-1451] Bug fix of Embedding %html with...
GitHub user astroshim opened a pull request:
https://github.com/apache/zeppelin/pull/1433
[ZEPPELIN-1451] Bug fix of Embedding %html within %table.
### What is this PR for?
This PR fixes the bug of Embedding %html within %table.
It doesn't print properly when the `%html` is the first line.
### What type of PR is it?
Bug Fix
### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-1451
### How should this be tested?
Run following code in your paragraph.
```
print(s"""%table
name\tsize\tquantity
%html <img src='https://upload.wikimedia.org/wikipedia/commons/thumb/6/6f/Sun_symbol.svg/50px-Sun_symbol.svg.png' />sun\t100\t50
%html <img src='https://upload.wikimedia.org/wikipedia/commons/thumb/2/2a/Moon_symbol_crescent.svg/25px-Moon_symbol_crescent.svg.png' />moon\t10\t20""")
```
### Screenshots (if appropriate)
- before
- after
### 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-1451
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/1433.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 #1433
----
commit f3d11a07a9e3ef405d2c7376f802c92a14ce6788
Author: astroshim <hs...@nflabs.com>
Date: 2016-09-18T13:26:13Z
add checking the type should be detected.
----
---
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 #1433: [ZEPPELIN-1451] Bug fix of Embedding %html within %tab...
Posted by astroshim <gi...@git.apache.org>.
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1433
@Leemoonsoo Thank you for pointing out the problem. Let me check.
---
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 #1433: [ZEPPELIN-1451] Bug fix of Embedding %html within %tab...
Posted by corneadoug <gi...@git.apache.org>.
Github user corneadoug commented on the issue:
https://github.com/apache/zeppelin/pull/1433
@astroshim CI failure is irrelevant
@Leemoonsoo any feedback?
---
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 #1433: [ZEPPELIN-1451] Bug fix of Embedding %html within %tab...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/1433
Now CI on master has fixed. @astroshim Could you rebase and see if it pass the test?
---
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 #1433: [ZEPPELIN-1451] Bug fix of Embedding %html within %tab...
Posted by astroshim <gi...@git.apache.org>.
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1433
I did rebase and CI has passed. 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 pull request #1433: [ZEPPELIN-1451] Bug fix of Embedding %html with...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/zeppelin/pull/1433
---
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 #1433: [ZEPPELIN-1451] Bug fix of Embedding %html within %tab...
Posted by corneadoug <gi...@git.apache.org>.
Github user corneadoug commented on the issue:
https://github.com/apache/zeppelin/pull/1433
@astroshim Tested, nice fix. LGTM
Can you rebase to have a green CI?
@Leemoonsoo @jongyoul @felixcheung Can one of you guys take a quick look at the code?
---
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 #1433: [ZEPPELIN-1451] Bug fix of Embedding %html within %tab...
Posted by astroshim <gi...@git.apache.org>.
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1433
@Leemoonsoo CI has passed, 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 #1433: [ZEPPELIN-1451] Bug fix of Embedding %html within %tab...
Posted by astroshim <gi...@git.apache.org>.
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1433
@Leemoonsoo ping
---
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 #1433: [ZEPPELIN-1451] Bug fix of Embedding %html within %tab...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/1433
Merge if there're 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 issue #1433: [ZEPPELIN-1451] Bug fix of Embedding %html within %tab...
Posted by astroshim <gi...@git.apache.org>.
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1433
@corneadoug Thank you for your review. I just re-based.
---
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 #1433: [ZEPPELIN-1451] Bug fix of Embedding %html with...
Posted by astroshim <gi...@git.apache.org>.
Github user astroshim closed the pull request at:
https://github.com/apache/zeppelin/pull/1433
---
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 #1433: [ZEPPELIN-1451] Bug fix of Embedding %html with...
Posted by astroshim <gi...@git.apache.org>.
GitHub user astroshim reopened a pull request:
https://github.com/apache/zeppelin/pull/1433
[ZEPPELIN-1451] Bug fix of Embedding %html within %table.
### What is this PR for?
This PR fixes the bug of Embedding %html within %table.
It doesn't print properly when the `%html` is the first line.
### What type of PR is it?
Bug Fix
### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-1451
### How should this be tested?
Run following code in your paragraph.
```
print(s"""%table
name\tsize\tquantity
%html <img src='https://upload.wikimedia.org/wikipedia/commons/thumb/6/6f/Sun_symbol.svg/50px-Sun_symbol.svg.png' />sun\t100\t50
%html <img src='https://upload.wikimedia.org/wikipedia/commons/thumb/2/2a/Moon_symbol_crescent.svg/25px-Moon_symbol_crescent.svg.png' />moon\t10\t20""")
```
### Screenshots (if appropriate)
- before
- after
### 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-1451
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/1433.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 #1433
----
commit f3d11a07a9e3ef405d2c7376f802c92a14ce6788
Author: astroshim <hs...@nflabs.com>
Date: 2016-09-18T13:26:13Z
add checking the type should be detected.
commit 7724afdd0fa241a999a8023f05539f5c1e50c83b
Author: astroshim <hs...@nflabs.com>
Date: 2016-09-21T12:44:30Z
Merge branch 'master' into ZEPPELIN-1451
commit a7a88e6e88a88d95e130d02d12310b234ad89787
Author: astroshim <hs...@nflabs.com>
Date: 2016-09-29T07:41:35Z
Merge branch 'master' of https://github.com/apache/zeppelin into ZEPPELIN-1451
commit 89a40b33dd11534ab44bd840774ecb669d365b5e
Author: astroshim <hs...@nflabs.com>
Date: 2016-09-29T12:49:30Z
change condition of parsing
commit 1d0ab93d266cc38c2758491baad6be1a3505010d
Author: astroshim <hs...@nflabs.com>
Date: 2016-09-30T02:21:21Z
add testcase
----
---
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 #1433: [ZEPPELIN-1451] Bug fix of Embedding %html within %tab...
Posted by astroshim <gi...@git.apache.org>.
Github user astroshim commented on the issue:
https://github.com/apache/zeppelin/pull/1433
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 #1433: [ZEPPELIN-1451] Bug fix of Embedding %html within %tab...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:
https://github.com/apache/zeppelin/pull/1433
@astroshim Currently only the last display system directive (%text, %html, ...) in the output is effective. Which is behavior of both InterpreterResult (static output) and InterpreterOutput (streaming output).
This patch fixes problem but break current behavior. That's why following test is failing on CI.
```
Results :
Failed tests:
InterpreterOutputTest.testType:92 expected:<HTML> but was:<TEXT>
Tests run: 68, Failures: 1, Errors: 0, Skipped: 0
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary:
[INFO]
[INFO] Zeppelin ........................................... SUCCESS [ 7.135 s]
[INFO] Zeppelin: Interpreter .............................. FAILURE [02:08 min]
```
@astroshim What do you think? will there a easy way to fix the problem without changing current behavior? (pass unittest without changing test)
---
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.
---