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/12/08 03:47:02 UTC

[GitHub] zeppelin pull request #1735: [ZEPPELIN-1767] Blank paragraph should not be r...

GitHub user astroshim opened a pull request:

    https://github.com/apache/zeppelin/pull/1735

    [ZEPPELIN-1767] Blank paragraph should not be run.

    ### What is this PR for?
    This PR fixes not to run blank paragraph.
    
    
    ### What type of PR is it?
    Improvement
    
    
    ### What is the Jira issue?
    https://issues.apache.org/jira/browse/ZEPPELIN-1767
    
    
    ### How should this be tested?
    please run blank paragraph.
    
    
    ### Screenshots (if appropriate)
    - before
    ![12 -08-2016 12-41-59](https://cloud.githubusercontent.com/assets/3348133/20996914/5d568e92-bd44-11e6-9462-aad47e4c08ca.gif)
    
    
    ### 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 skipBlankParagraph

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zeppelin/pull/1735.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 #1735
    
----
commit 35f049790131c0a0f15fccff74f5733a1f36b822
Author: astroshim <hs...@zepl.com>
Date:   2016-12-08T03:29:59Z

    skip blank to run paragraph.

----


---
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 #1735: [ZEPPELIN-1767] Blank paragraph should not be run.

Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:

    https://github.com/apache/zeppelin/pull/1735
  
    LGTM \U0001f44d 
    Merge if there are no more comments. 


---
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 #1735: [ZEPPELIN-1767] Blank paragraph should not be r...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/zeppelin/pull/1735


---
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 #1735: [ZEPPELIN-1767] Blank paragraph should not be run.

Posted by astroshim <gi...@git.apache.org>.
Github user astroshim commented on the issue:

    https://github.com/apache/zeppelin/pull/1735
  
    CI is green 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 issue #1735: [ZEPPELIN-1767] Blank paragraph should not be run.

Posted by astroshim <gi...@git.apache.org>.
Github user astroshim commented on the issue:

    https://github.com/apache/zeppelin/pull/1735
  
    Testcase also added. @AhyoungRyu 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 pull request #1735: [ZEPPELIN-1767] Blank paragraph should not be r...

Posted by astroshim <gi...@git.apache.org>.
Github user astroshim closed the pull request at:

    https://github.com/apache/zeppelin/pull/1735


---
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 #1735: [ZEPPELIN-1767] Blank paragraph should not be r...

Posted by astroshim <gi...@git.apache.org>.
GitHub user astroshim reopened a pull request:

    https://github.com/apache/zeppelin/pull/1735

    [ZEPPELIN-1767] Blank paragraph should not be run.

    ### What is this PR for?
    This PR fixes not to run blank paragraph.
    
    
    ### What type of PR is it?
    Improvement
    
    
    ### What is the Jira issue?
    https://issues.apache.org/jira/browse/ZEPPELIN-1767
    
    
    ### How should this be tested?
    please run blank paragraph.
    
    
    ### Screenshots (if appropriate)
    - before
    ![12 -08-2016 12-41-59](https://cloud.githubusercontent.com/assets/3348133/20996914/5d568e92-bd44-11e6-9462-aad47e4c08ca.gif)
    
    
    ### 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 skipBlankParagraph

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zeppelin/pull/1735.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 #1735
    
----
commit 35f049790131c0a0f15fccff74f5733a1f36b822
Author: astroshim <hs...@zepl.com>
Date:   2016-12-08T03:29:59Z

    skip blank to run paragraph.

commit b7c9d7237350b51790c3ee74776827103b7a887c
Author: astroshim <hs...@zepl.com>
Date:   2016-12-08T10:10:22Z

    fix typo

commit c80fbebfcbea0cd4cbe812bcdde014b13711baf4
Author: astroshim <hs...@zepl.com>
Date:   2016-12-10T16:37:04Z

    fix restapi issue

commit 68ca222ed68699001d33c558db02a943b0c589bf
Author: astroshim <hs...@zepl.com>
Date:   2016-12-11T08:54:31Z

    add testcase

commit 14f9714aa55f7dd21e3c9780c20a1f5cde0a444e
Author: astroshim <hs...@zepl.com>
Date:   2016-12-11T13:49:00Z

    Merge branch 'master' into skipBlankParagraph

commit ccd99823d460a7cf07d181f2aecfb5d45f879670
Author: astroshim <hs...@zepl.com>
Date:   2016-12-12T15:09:45Z

    fix 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 #1735: [ZEPPELIN-1767] Blank paragraph should not be run.

Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:

    https://github.com/apache/zeppelin/pull/1735
  
    @astroshim Thanks for your prompt update. Tested and it works well! 
    But sadly 3 of CI tests are failed likw below and looks relevant with this change. Could you take a look this again please?
    ```
    Results :
    
    Failed tests: 
      NoteTest.runNormalTest:80 
    No interactions wanted here:
    -> at org.apache.zeppelin.notebook.NoteTest.runNormalTest(NoteTest.java:80)
    But found this interaction on mock 'interpreterFactory':
    -> at org.apache.zeppelin.notebook.Paragraph.isValidInterpreter(Paragraph.java:598)
    ***
    For your reference, here is the list of all invocations ([?] - means unverified).
    1. [?]-> at org.apache.zeppelin.notebook.Paragraph.isValidInterpreter(Paragraph.java:598)
    2. [?]-> at org.apache.zeppelin.notebook.Note.run(Note.java:550)
    ```


---
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 #1735: [ZEPPELIN-1767] Blank paragraph should not be run.

Posted by astroshim <gi...@git.apache.org>.
Github user astroshim commented on the issue:

    https://github.com/apache/zeppelin/pull/1735
  
    I just fixed typo.
    Thanks @rhymoid and @cloverhearts to review this PR. 


---
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 #1735: [ZEPPELIN-1767] Blank paragraph should not be r...

Posted by astroshim <gi...@git.apache.org>.
GitHub user astroshim reopened a pull request:

    https://github.com/apache/zeppelin/pull/1735

    [ZEPPELIN-1767] Blank paragraph should not be run.

    ### What is this PR for?
    This PR fixes not to run blank paragraph.
    
    
    ### What type of PR is it?
    Improvement
    
    
    ### What is the Jira issue?
    https://issues.apache.org/jira/browse/ZEPPELIN-1767
    
    
    ### How should this be tested?
    please run blank paragraph.
    
    
    ### Screenshots (if appropriate)
    - before
    ![12 -08-2016 12-41-59](https://cloud.githubusercontent.com/assets/3348133/20996914/5d568e92-bd44-11e6-9462-aad47e4c08ca.gif)
    
    
    ### 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 skipBlankParagraph

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zeppelin/pull/1735.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 #1735
    
----
commit 35f049790131c0a0f15fccff74f5733a1f36b822
Author: astroshim <hs...@zepl.com>
Date:   2016-12-08T03:29:59Z

    skip blank to run paragraph.

commit b7c9d7237350b51790c3ee74776827103b7a887c
Author: astroshim <hs...@zepl.com>
Date:   2016-12-08T10:10:22Z

    fix typo

commit c80fbebfcbea0cd4cbe812bcdde014b13711baf4
Author: astroshim <hs...@zepl.com>
Date:   2016-12-10T16:37:04Z

    fix restapi issue

commit 68ca222ed68699001d33c558db02a943b0c589bf
Author: astroshim <hs...@zepl.com>
Date:   2016-12-11T08:54:31Z

    add testcase

commit 14f9714aa55f7dd21e3c9780c20a1f5cde0a444e
Author: astroshim <hs...@zepl.com>
Date:   2016-12-11T13:49:00Z

    Merge branch 'master' into skipBlankParagraph

commit ccd99823d460a7cf07d181f2aecfb5d45f879670
Author: astroshim <hs...@zepl.com>
Date:   2016-12-12T15:09:45Z

    fix 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 pull request #1735: [ZEPPELIN-1767] Blank paragraph should not be r...

Posted by astroshim <gi...@git.apache.org>.
Github user astroshim closed the pull request at:

    https://github.com/apache/zeppelin/pull/1735


---
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 #1735: [ZEPPELIN-1767] Blank paragraph should not be run.

Posted by cloverhearts <gi...@git.apache.org>.
Github user cloverhearts commented on the issue:

    https://github.com/apache/zeppelin/pull/1735
  
    I think we need this feature.
    and on focus to good fixed.


---
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 #1735: [ZEPPELIN-1767] Blank paragraph should not be run.

Posted by astroshim <gi...@git.apache.org>.
Github user astroshim commented on the issue:

    https://github.com/apache/zeppelin/pull/1735
  
    Thank you @AhyoungRyu ! I didn't catch the 3rd.
    I just fix that and let me add testcase for this.



---
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 #1735: [ZEPPELIN-1767] Blank paragraph should not be run.

Posted by AhyoungRyu <gi...@git.apache.org>.
Github user AhyoungRyu commented on the issue:

    https://github.com/apache/zeppelin/pull/1735
  
    @astroshim Good catch! Tested and it was working well as expected. 
    AFAIK there are 3 way to run a paragraph in Zeppelin note like below 
    
    1. \w keyboard shortcut `Shift + Enter` : we can't run the blank paragraph by default.
    2. \w icon button: your PR disable this one :)
    3. \w RestApi [like this](https://zeppelin.apache.org/docs/0.7.0-SNAPSHOT/rest-api/rest-notebook.html#run-a-paragraph-asynchronously) : I was able to run the blank paragraph 
    
    I think we need to block 3rd case as well. What do you think? 



---
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.
---