Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.6.0
    • Fix Version/s: 0.7.0
    • Component/s: zeppelin-server
    • Labels:
      None

      Description

      was looking for a api to clear result in a paragraph.
      The notebook which i executed got more data which affected the notebook loading.

      Creating this defect because was not able to find documentation of the api at https://zeppelin.apache.org/docs/0.6.0/rest-api/rest-notebook.html.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/zeppelin/pull/1565
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user tae-jun commented on the issue:

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

          @anthonycorbacho @khalidhuseynov Thanks for the comment! I agree with you guys.

          This is a breaking change, so yeah we must be careful. So, if we proceed this, new API URL's prefix will be `v1/api/notebook`. And keep legacy API under `api/notebook` for backward compatibility. This will be the start of versioning.

          I think we should design REST API together. I will create a proposal on JIRA soon, please review it when the time comes!

          It will be harder to fix later. So, let's start now! 😄

          Show
          githubbot ASF GitHub Bot added a comment - Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1565 @anthonycorbacho @khalidhuseynov Thanks for the comment! I agree with you guys. This is a breaking change, so yeah we must be careful. So, if we proceed this, new API URL's prefix will be `v1/api/notebook`. And keep legacy API under `api/notebook` for backward compatibility. This will be the start of versioning. I think we should design REST API together. I will create a proposal on JIRA soon, please review it when the time comes! It will be harder to fix later. So, let's start now! 😄
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user khalidhuseynov commented on the issue:

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

          @tae-jun thanks for summarizing and giving feedback on REST API. Although we can't address all at once, but I think it deserves an issue with above content and feedback logged in there

          Show
          githubbot ASF GitHub Bot added a comment - Github user khalidhuseynov commented on the issue: https://github.com/apache/zeppelin/pull/1565 @tae-jun thanks for summarizing and giving feedback on REST API. Although we can't address all at once, but I think it deserves an issue with above content and feedback logged in there
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user anthonycorbacho commented on the issue:

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

          @tae-jun This is a great sum of the status of the rest api in zeppelin, and i think we all agree that currently zeppelin doenst follow rest standard (just by looking at the endpoint + response (mostly 500 ...)).

          But you have to consider that going on the path of refacto this logic will be a huge task, not only in backend but you need to understand that a lot of users are using this api already changing the hole api will brings a lot of breaking change in they current workflow. So we need to be careful here.

          Show
          githubbot ASF GitHub Bot added a comment - Github user anthonycorbacho commented on the issue: https://github.com/apache/zeppelin/pull/1565 @tae-jun This is a great sum of the status of the rest api in zeppelin, and i think we all agree that currently zeppelin doenst follow rest standard (just by looking at the endpoint + response (mostly 500 ...)). But you have to consider that going on the path of refacto this logic will be a huge task, not only in backend but you need to understand that a lot of users are using this api already changing the hole api will brings a lot of breaking change in they current workflow. So we need to be careful here.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user tae-jun commented on the issue:

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

          @minahlee Oh, in that context, it makes sense. LGTM.

          But, after looking all around Notebook REST API, it works well now but it doesn't seem RESTful. I summarized current list of REST API:

          • `GET` /
          • `POST` /
          • `GET` /[noteId]
          • `POST` /[noteId]
          • `DELETE` /[noteId]
          • `GET` /job/[noteId]
          • `POST` /job/[noteId]
          • `DELETE` /job/[noteId]
          • `GET` /job/[noteId]/[paragraphId]
          • `POST` /job/[noteId]/[paragraphId]
          • `DELETE` /job/[noteId]/[paragraphId]
          • `POST` /run/[noteId]/[paragraphId]
          • `GET` /cron/[noteId]
          • `POST` /cron/[noteId]
          • `DELETE` /cron/[noteId]
          • `GET` /search?q=[query]
          • `POST` /[noteId]/paragraph
          • `GET` /[noteId]/paragraph/[paragraphId]
          • `DELETE` /[noteId]/paragraph/[paragraphId]
          • `POST` /[noteId]/paragraph/[paragraphId]/move/[newIndex]
          • `GET` /export/[noteId]
          • `POST` /import

          A good REST API is API that doesn't need a documentation, which means users can understands only with HTTP method and URL. So, it should be simple. However, I think some of current API may need to be changed.

          REST API is UI for developers, so should be well designed. There are problems I can see now:

          • Need to organize resources of Zeppelin: hierarchy of resources, and what they have; For example:
          • Need hierarchical URL design: `notebook/:noteId/paragraphs/:paragraphId/ {title|text|result|job}

            `

          • Do jobs belong to notebook or paragraph?
          • Use plural
          • Use nouns as far as you can, no verbs. Make use of HTTP methods. URL should point resources.
          • For in the case of a request that doesn't fit CRUD(Create(POST), Read(GET), Update(PUT|PATCH), Detele(DELETE)), we need a rule for this. Heroku deals with this by placing actions for the resource under `/actions`. For example, in our case, run all paragraphs can be represented as `POST notebook/:noteId/actions/run`
          • Need API versioning

          Zeppelin needs more REST API, so eventually, it will be getting bigger. I think it's time to organize REST APIs. After all of this, the REST API documentation should also be updated. Not by listing actions what you can, just by listing APIs. This is easier to read since users can know what they can do with resources.

          Would you mind if I open an issue for this?

          Show
          githubbot ASF GitHub Bot added a comment - Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1565 @minahlee Oh, in that context, it makes sense. LGTM. But, after looking all around Notebook REST API, it works well now but it doesn't seem RESTful. I summarized current list of REST API: `GET` / `POST` / `GET` / [noteId] `POST` / [noteId] `DELETE` / [noteId] `GET` /job/ [noteId] `POST` /job/ [noteId] `DELETE` /job/ [noteId] `GET` /job/ [noteId] / [paragraphId] `POST` /job/ [noteId] / [paragraphId] `DELETE` /job/ [noteId] / [paragraphId] `POST` /run/ [noteId] / [paragraphId] `GET` /cron/ [noteId] `POST` /cron/ [noteId] `DELETE` /cron/ [noteId] `GET` /search?q= [query] `POST` / [noteId] /paragraph `GET` / [noteId] /paragraph/ [paragraphId] `DELETE` / [noteId] /paragraph/ [paragraphId] `POST` / [noteId] /paragraph/ [paragraphId] /move/ [newIndex] `GET` /export/ [noteId] `POST` /import A good REST API is API that doesn't need a documentation, which means users can understands only with HTTP method and URL. So, it should be simple. However, I think some of current API may need to be changed. REST API is UI for developers, so should be well designed. There are problems I can see now: Need to organize resources of Zeppelin: hierarchy of resources, and what they have; For example: Need hierarchical URL design: `notebook/:noteId/paragraphs/:paragraphId/ {title|text|result|job} ` Do jobs belong to notebook or paragraph? Use plural Use nouns as far as you can, no verbs. Make use of HTTP methods. URL should point resources. For in the case of a request that doesn't fit CRUD(Create(POST), Read(GET), Update(PUT|PATCH), Detele(DELETE)), we need a rule for this. Heroku deals with this by placing actions for the resource under `/actions`. For example, in our case, run all paragraphs can be represented as `POST notebook/:noteId/actions/run` Need API versioning Zeppelin needs more REST API, so eventually, it will be getting bigger. I think it's time to organize REST APIs. After all of this, the REST API documentation should also be updated. Not by listing actions what you can, just by listing APIs. This is easier to read since users can know what they can do with resources. Would you mind if I open an issue for this?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user minahlee commented on the issue:

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

          @tae-jun the reason I use `PUT` is that `clearAllParagraphOutput` method is not really deleting `result` field from paragraph, but updating `result` field to empty.
          I was considering using `DELETE` but I didn't because it can confuse user that it will literally 'DELETE' result field.

          What do you think?

          Show
          githubbot ASF GitHub Bot added a comment - Github user minahlee commented on the issue: https://github.com/apache/zeppelin/pull/1565 @tae-jun the reason I use `PUT` is that `clearAllParagraphOutput` method is not really deleting `result` field from paragraph, but updating `result` field to empty. I was considering using `DELETE` but I didn't because it can confuse user that it will literally 'DELETE' result field. What do you think?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user tae-jun commented on the issue:

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

          @minahlee I have a question about REST API style. Is there any purpose that you used `PUT` method?

          In my opinion, `DELETE` method is more proper since it does not `PUT` `clear` under a `noteId`. This action `DELETE`s `noteId`'s `outputs`. So, this is API style I suggest:

          ```
          DELETE

          {noteId}/outputs
          ```

          And also maybe we can implement one paragraph output deletion in the future like:

          ```
          DELETE {noteId}

          /outputs/

          {paragraphId}

          ```

          Show
          githubbot ASF GitHub Bot added a comment - Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1565 @minahlee I have a question about REST API style. Is there any purpose that you used `PUT` method? In my opinion, `DELETE` method is more proper since it does not `PUT` `clear` under a `noteId`. This action `DELETE`s `noteId`'s `outputs`. So, this is API style I suggest: ``` DELETE {noteId}/outputs ``` And also maybe we can implement one paragraph output deletion in the future like: ``` DELETE {noteId} /outputs/ {paragraphId} ```
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user minahlee commented on the issue:

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

          @khalidhuseynov Thank you for quick response!
          Merging if there is no more discussion.

          Show
          githubbot ASF GitHub Bot added a comment - Github user minahlee commented on the issue: https://github.com/apache/zeppelin/pull/1565 @khalidhuseynov Thank you for quick response! Merging if there is no more discussion.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user khalidhuseynov commented on the issue:

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

          @minahlee looks much better now! thanks for addressing the issue and LGTM

          Show
          githubbot ASF GitHub Bot added a comment - Github user khalidhuseynov commented on the issue: https://github.com/apache/zeppelin/pull/1565 @minahlee looks much better now! thanks for addressing the issue and LGTM
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user minahlee commented on the issue:

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

          @khalidhuseynov I updated rest api to do privilege check. Could you help review?

          Show
          githubbot ASF GitHub Bot added a comment - Github user minahlee commented on the issue: https://github.com/apache/zeppelin/pull/1565 @khalidhuseynov I updated rest api to do privilege check. Could you help review?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user Leemoonsoo commented on the issue:

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

          Tested and LGTM

          Show
          githubbot ASF GitHub Bot added a comment - Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/1565 Tested and LGTM
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user tae-jun commented on the issue:

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

          @minahlee Awesome! I just changed status to `IN PROGRESS` now. Thanks 👍

          Show
          githubbot ASF GitHub Bot added a comment - Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1565 @minahlee Awesome! I just changed status to `IN PROGRESS` now. Thanks 👍
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user minahlee commented on the issue:

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

          @tae-jun I just added your name to contributor list so you will be able to set yourself as assignee from now.

          Show
          githubbot ASF GitHub Bot added a comment - Github user minahlee commented on the issue: https://github.com/apache/zeppelin/pull/1565 @tae-jun I just added your name to contributor list so you will be able to set yourself as assignee from now.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user tae-jun commented on the issue:

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

          Thanks @minahlee! I appreciate your kind comment 😄

          I created a ticket on JIRA: ZEPPELIN-1598(https://issues.apache.org/jira/browse/ZEPPELIN-1598)

          But, I don't know how to set myself to assignee, and change status to `PROGRESS`. I guess I don't have a permission to do that 😢

          I will check #1567. Thanks again!

          Show
          githubbot ASF GitHub Bot added a comment - Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1565 Thanks @minahlee! I appreciate your kind comment 😄 I created a ticket on JIRA: ZEPPELIN-1598 ( https://issues.apache.org/jira/browse/ZEPPELIN-1598 ) But, I don't know how to set myself to assignee, and change status to `PROGRESS`. I guess I don't have a permission to do that 😢 I will check #1567. Thanks again!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user minahlee commented on the issue:

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

          Hi @tae-jun, thank you for your interest. I had to rebase this branch to resolve conflict so you will need too since it is based on mine.

          I am planning to finish this PR once #1567 merged so that I can handle permission issue. It will speed up merging process if you can review it (I am also going to review it as soon as all the tasks are done).

          About the jira, there is nothing stops you from creating one so feel free to create a ticket on Jira. And make sure that you assign your self on that task, and put `PROGRESS` label so everyone can see that you are working on it. And for the PR, I suggest you to do the cherry pick after #1567, #1565 merged, otherwise it will cause some confusion for the ones who don't know the context.

          Show
          githubbot ASF GitHub Bot added a comment - Github user minahlee commented on the issue: https://github.com/apache/zeppelin/pull/1565 Hi @tae-jun, thank you for your interest. I had to rebase this branch to resolve conflict so you will need too since it is based on mine. I am planning to finish this PR once #1567 merged so that I can handle permission issue. It will speed up merging process if you can review it (I am also going to review it as soon as all the tasks are done). About the jira, there is nothing stops you from creating one so feel free to create a ticket on Jira. And make sure that you assign your self on that task, and put `PROGRESS` label so everyone can see that you are working on it. And for the PR, I suggest you to do the cherry pick after #1567, #1565 merged, otherwise it will cause some confusion for the ones who don't know the context.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user tae-jun commented on the issue:

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

          Hi @minahlee This is a cool feature! I have an experience that deleted output through editing note.json because of huge output stopping Zeppelin web.

          By the way, I've wanted a renaming folder feature. So as soon as I saw this PR, I implemented renaming note, and I am almost done with implementing renaming folder! (I saw you are planning this after coding, sorry 😢) This is a really good example for implementing it!

          So, the question is this:
          My branch is based on your PR's branch. How should I create JIRA issue? As well as PR.
          Is it easy to wait to merge this PR?

          Show
          githubbot ASF GitHub Bot added a comment - Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1565 Hi @minahlee This is a cool feature! I have an experience that deleted output through editing note.json because of huge output stopping Zeppelin web. By the way, I've wanted a renaming folder feature. So as soon as I saw this PR, I implemented renaming note, and I am almost done with implementing renaming folder! (I saw you are planning this after coding, sorry 😢) This is a really good example for implementing it! So, the question is this: My branch is based on your PR's branch. How should I create JIRA issue? As well as PR. Is it easy to wait to merge this PR?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user minahlee commented on the issue:

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

          @khalidhuseynov Thank you for review. I made "clear paragraph output" in main page for the case that note's output data is too big so it causes Zeppelin fail to load note and hang.
          Here are tickets reporting this issue: ZEPPELIN-1564(https://issues.apache.org/jira/browse/ZEPPELIN-1564), ZEPPELIN-1491(https://issues.apache.org/jira/browse/ZEPPELIN-1491)

          The more optimal solution would be improving data loading and rendering itself. We might not need "clear paragraph output" button once we improve this performance issue.

          I believe having delete button on the main page is beneficial in terms of UX in case user wants to delete multiple notes. They won't need to navigate into each note to delete them. I am planning to implement remove folder & edit folder name in main page after this PR is merged.

          Show
          githubbot ASF GitHub Bot added a comment - Github user minahlee commented on the issue: https://github.com/apache/zeppelin/pull/1565 @khalidhuseynov Thank you for review. I made "clear paragraph output" in main page for the case that note's output data is too big so it causes Zeppelin fail to load note and hang. Here are tickets reporting this issue: ZEPPELIN-1564 ( https://issues.apache.org/jira/browse/ZEPPELIN-1564 ), ZEPPELIN-1491 ( https://issues.apache.org/jira/browse/ZEPPELIN-1491 ) The more optimal solution would be improving data loading and rendering itself. We might not need "clear paragraph output" button once we improve this performance issue. I believe having delete button on the main page is beneficial in terms of UX in case user wants to delete multiple notes. They won't need to navigate into each note to delete them. I am planning to implement remove folder & edit folder name in main page after this PR is merged.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user minahlee opened a pull request:

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

          ZEPPELIN-1564 Enable note deletion and paragraph output clear from main page

              1. What is this PR for?
          • Enables removing note and clear all paragraph's output from Zeppelin main page.
          • Add rest api for clearing all paragraph output
            Next possible improvement can be removing notes in folder level.
              1. What type of PR is it?
                Improvement
              1. What is the Jira issue?
                ZEPPELIN-1564(https://issues.apache.org/jira/browse/ZEPPELIN-1564)
              1. Screenshots (if appropriate)
                ![oct-27-2016 18-26-03](https://cloud.githubusercontent.com/assets/8503346/19761938/e013ea02-9c72-11e6-9a08-0a70aca145d2.gif)
              1. Questions:
          • Does the licenses files need update? no
          • Is there breaking changes for older versions? no
          • Does this needs documentation? yes

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/minahlee/zeppelin ZEPPELIN-1564

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

          https://github.com/apache/zeppelin/pull/1565.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 #1565


          commit cb70a6e9593c2dc748e04dc2d43ebac9c1b52a5a
          Author: Mina Lee <minalee@apache.org>
          Date: 2016-10-27T07:50:18Z

          Clear all paragraphs and remove note from main page

          commit 8b25f27ecc6c2674f50377f7828c9c4d723db012
          Author: Mina Lee <minalee@apache.org>
          Date: 2016-10-27T08:07:29Z

          Add clearAllParagraphOutput unit test

          commit f0962579e143f47946e8fa64f247abe91373f2f1
          Author: Mina Lee <minalee@apache.org>
          Date: 2016-10-27T08:53:33Z

          Add rest api for clear all paragraph result and add test

          commit 4f61c31c5b8e8ff0855bfe0e272bd7cd92bd1f39
          Author: Mina Lee <minalee@apache.org>
          Date: 2016-10-27T09:10:58Z

          Add rest api endpoint for clear paragraph result to document

          commit 70eb294e26e41da953385ff94379fe3e7cec4716
          Author: Mina Lee <minalee@apache.org>
          Date: 2016-10-27T09:14:42Z

          Remove unused import


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user minahlee opened a pull request: https://github.com/apache/zeppelin/pull/1565 ZEPPELIN-1564 Enable note deletion and paragraph output clear from main page What is this PR for? Enables removing note and clear all paragraph's output from Zeppelin main page. Add rest api for clearing all paragraph output Next possible improvement can be removing notes in folder level. What type of PR is it? Improvement What is the Jira issue? ZEPPELIN-1564 ( https://issues.apache.org/jira/browse/ZEPPELIN-1564 ) Screenshots (if appropriate) ! [oct-27-2016 18-26-03] ( https://cloud.githubusercontent.com/assets/8503346/19761938/e013ea02-9c72-11e6-9a08-0a70aca145d2.gif ) Questions: Does the licenses files need update? no Is there breaking changes for older versions? no Does this needs documentation? yes You can merge this pull request into a Git repository by running: $ git pull https://github.com/minahlee/zeppelin ZEPPELIN-1564 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/zeppelin/pull/1565.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 #1565 commit cb70a6e9593c2dc748e04dc2d43ebac9c1b52a5a Author: Mina Lee <minalee@apache.org> Date: 2016-10-27T07:50:18Z Clear all paragraphs and remove note from main page commit 8b25f27ecc6c2674f50377f7828c9c4d723db012 Author: Mina Lee <minalee@apache.org> Date: 2016-10-27T08:07:29Z Add clearAllParagraphOutput unit test commit f0962579e143f47946e8fa64f247abe91373f2f1 Author: Mina Lee <minalee@apache.org> Date: 2016-10-27T08:53:33Z Add rest api for clear all paragraph result and add test commit 4f61c31c5b8e8ff0855bfe0e272bd7cd92bd1f39 Author: Mina Lee <minalee@apache.org> Date: 2016-10-27T09:10:58Z Add rest api endpoint for clear paragraph result to document commit 70eb294e26e41da953385ff94379fe3e7cec4716 Author: Mina Lee <minalee@apache.org> Date: 2016-10-27T09:14:42Z Remove unused import
          Hide
          moon Lee moon soo added a comment -

          This issue is related to ZEPPELIN-1491

          Show
          moon Lee moon soo added a comment - This issue is related to ZEPPELIN-1491

            People

            • Assignee:
              Mina Lee Mina Lee
              Reporter:
              PankajSingh Pankaj Singh
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development