You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@zeppelin.apache.org by onkarshedge <gi...@git.apache.org> on 2016/06/10 10:42:32 UTC

[GitHub] zeppelin pull request #989: Added ipfsnotebookrepo [ Zeppelin-683 WIP ]

GitHub user onkarshedge opened a pull request:

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

     Added ipfsnotebookrepo [ Zeppelin-683 WIP ]

    ### What is this PR for?
    IPFSNoteBookRepo IPFS storage and sharing zeppelin notes.
    
    
    ### What type of PR is it?
     Feature 
    
    ### Todos
    * [API Multiaddress to connect to ipfs daemon get from configuration ]
    * [Import Note from other peers when a ipfs multihash is given ] 
    * [UI support to import the Note when hash is given as input] 
    * [Testing ] 
    
    ### What is the Jira issue?
    * [Jira issue](https://issues.apache.org/jira/browse/ZEPPELIN-683)
    
    ### How should this be tested?
    [Google Document](https://docs.google.com/document/d/1eYjQL0t7VlBTRMYoA0nWRJAnZ6RHUF7S7G3rx9xH5Pc/edit?usp=sharing)
    
    ### Screenshots (if appropriate)
    [Google Document](https://docs.google.com/document/d/1eYjQL0t7VlBTRMYoA0nWRJAnZ6RHUF7S7G3rx9xH5Pc/edit?usp=sharing)
    
    ### Questions:
    * Does the licenses files need update?
    * Is there breaking changes for older versions?
    * Does this needs documentation?
    


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

    $ git pull https://github.com/onkarshedge/incubator-zeppelin ipfs

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

    https://github.com/apache/zeppelin/pull/989.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 #989
    
----
commit d4eb14b47221b10e6ad2eea482f77c23caa81255
Author: onkarshedge <sh...@gmail.com>
Date:   2016-06-10T07:55:59Z

     Added ipfsnotebookrepo

----


---
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 #989: Added ipfsnotebookrepo [ Zeppelin-683 WIP ]

Posted by onkarshedge <gi...@git.apache.org>.

Github user onkarshedge commented on the issue:

    https://github.com/apache/zeppelin/pull/989
  
    @bzz I had a doubt about versioning approach.
    Currently I am saving(ipfs add 'note') the note in IPFS for every call to save function. This gives me a new multihash on saving. So for versioning I have to save the multihash with time. The `GitNotebookRepo` has a checkpoint which corresponds to git commit. Here in ipfs each call to ipfs add is like git commit.  So the List<Rev> from history function will be large because save() method is called multiple times.
    So  the List<Rev> should contain only those few multihashes   which user wants to choose.


---
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 #989: Added ipfsnotebookrepo [ Zeppelin-683 WIP ]

Posted by bzz <gi...@git.apache.org>.

Github user bzz commented on the issue:

    https://github.com/apache/zeppelin/pull/989
  
    @onkarshedge great progress, thank you for keeping us posted! 
    
    Before moving further, on top of great suggestions from @corneadoug, there are few more things you also need to take care of:
    
     - Please make sure that `zeppelin-distribution/src/bin_license/LICENSE` includes all dependencies _AND all transitive dependencies_ that this changes bring to the project. You can see them with `mvn dependency:tree`
    
     - Please make sure that ALL your Java code follows the conventions from [project style guide](https://github.com/apache/zeppelin/blob/master/CONTRIBUTING.md#code-convention)
    
     - I noticed that CI is failing here. Could you, as an author of the PR, look into that and post there the reason of the failures here please? This would speed up the review and enable the reviewers to help you identify the reason faster. 


---
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 #989: Added ipfsnotebookrepo [ Zeppelin-683 WIP ]

Posted by onkarshedge <gi...@git.apache.org>.

Github user onkarshedge commented on the issue:

    https://github.com/apache/zeppelin/pull/989
  
    @bzz 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 #989: Added ipfsnotebookrepo [ Zeppelin-683 WIP ]

Posted by bzz <gi...@git.apache.org>.

Github user bzz commented on the issue:

    https://github.com/apache/zeppelin/pull/989
  
    Got it, thanks! Please submit another WIP pr with Bittorents support.
    
    Could you still follow https://github.com/apache/zeppelin/pull/989#issuecomment-230972882 and update this PR as soon as you get back to IPFS one?
    
    Thanks again!


---
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 #989: Added ipfsnotebookrepo [ Zeppelin-683 WIP ]

Posted by bzz <gi...@git.apache.org>.

Github user bzz commented on the issue:

    https://github.com/apache/zeppelin/pull/989
  
    Do you want to do that on the client-side, right in the browser? 
    
    I see what you mean - our existing impor function indeed fetches from URL and parse JSON right in the browser, but I do not think that such architecture will suit our case, as then the whole IPFS stack need to be run on the client. And it is not something that we want add here (we want  self-contained `IpfsNotebookRepo` implementation)  
    
    I think what you could do here is - create a new REST API endpoint, pass this hash (which is like an URL in our case) to this REST API, and make it delegate actual fetching logic to a method in `IpfsNotebookRepo` and then, on the client, just rear the response, as a JSON.
    
    This logic can be later used for BitTorrent notebooks storage support as well as simple URL case could be refactored to use it (but that is outside of the scope of this PR of course) 
    
    Let me introduce you to @corneadoug - he is known Zeppelin front-end ninja, whose knowledge of the client side codebase is by far greater than mine.
    
    @corneadoug Could you please help @onkarshedge and guide him on refactoring\updating `notenameImport.controller.js` in such way, that it could delegate hash->JSON resolution in case of notebook import to the backend API, for the case of importing notes from new IPFS storage by IPFS hash (which is like a bittorent address)?


---
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 #989: Added ipfsnotebookrepo [ Zeppelin-683 WIP ]

Posted by bzz <gi...@git.apache.org>.

Github user bzz commented on the issue:

    https://github.com/apache/zeppelin/pull/989
  
    ping, @onkarshedge are there any updates for the past week?


---
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 #989: Added ipfsnotebookrepo [ Zeppelin-683 ]

Posted by corneadoug <gi...@git.apache.org>.

Github user corneadoug commented on the issue:

    https://github.com/apache/zeppelin/pull/989
  
    Considering https://github.com/apache/zeppelin/pull/1304 is the import really needed?


---
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 #989: Added ipfsnotebookrepo [ Zeppelin-683 WIP ]

Posted by onkarshedge <gi...@git.apache.org>.

Github user onkarshedge commented on the issue:

    https://github.com/apache/zeppelin/pull/989
  
    @bzz 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.
---