You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by zjffdu <gi...@git.apache.org> on 2018/08/24 08:47:52 UTC
[GitHub] zeppelin pull request #3163: ZEPPELIN-2619. Save note in [Title].zpln instea...
GitHub user zjffdu opened a pull request:
https://github.com/apache/zeppelin/pull/3163
ZEPPELIN-2619. Save note in [Title].zpln instead of [NOTEID]/note.json
### What is this PR for?
A few sentences describing the overall goals of the pull request's commits.
First time? Check out the contributing guide - https://zeppelin.apache.org/contribution/contributions.html
### What type of PR is it?
[Bug Fix | Improvement | Feature | Documentation | Hot Fix | Refactoring]
### Todos
* [ ] - Task
### What is the Jira issue?
* Open an issue on Jira https://issues.apache.org/jira/browse/ZEPPELIN/
* Put link here, and add [ZEPPELIN-*Jira number*] in PR title, eg. [ZEPPELIN-533]
### How should this be tested?
* First time? Setup Travis CI as described on https://zeppelin.apache.org/contribution/contributions.html#continuous-integration
* Strongly recommended: add automated unit tests for any new or changed behavior
* Outline any manual steps to test the PR here.
### Screenshots (if appropriate)
### 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/zjffdu/zeppelin ZEPPELIN-2619
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zeppelin/pull/3163.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 #3163
----
commit 6d13b4a45a2cc11b646cffd14664d6b7bae676a1
Author: Jeff Zhang <zj...@...>
Date: 2018-07-06T07:01:44Z
ZEPPELIN-2619. Save note in [Title].zpln instead of [NOTEID]/note.json
----
---
[GitHub] zeppelin issue #3163: ZEPPELIN-2619. Save note in ${notename_noteid}.zpln in...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/3163
Thanks @mebelousov for verifying this PR, I have addressed the issue.
---
[GitHub] zeppelin issue #3163: [WIP] ZEPPELIN-2619. Save note in [Title].zpln instead...
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the issue:
https://github.com/apache/zeppelin/pull/3163
I have a very simple request about a format of `.zpln`. Could you please add version number of note format? e.g. `"version": "0"`. It would help to parse `.zpln` in the future
---
[GitHub] zeppelin pull request #3163: ZEPPELIN-2619. Save note in ${notename_noteid}....
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/zeppelin/pull/3163
---
[GitHub] zeppelin issue #3163: ZEPPELIN-2619. Save note in [Title].zpln instead of [N...
Posted by mebelousov <gi...@git.apache.org>.
Github user mebelousov commented on the issue:
https://github.com/apache/zeppelin/pull/3163
@zjffdu
I had a little test.
- Create note with name "Folder/Name 1"
- It was saved on disc correctly.
- Try to edit note name: folder path is absent.
---
[GitHub] zeppelin issue #3163: ZEPPELIN-2619. Save note in [Title].zpln instead of [N...
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the issue:
https://github.com/apache/zeppelin/pull/3163
Wow... it’s really too big. I understand the concept fully but need more explaination of whole changes.
---
[GitHub] zeppelin issue #3163: ZEPPELIN-2619. Save note in ${notename_noteid}.zpln in...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/3163
@mebelousov Do you see popup windows saying `Fail to create note, cause: Note name can not contain '..'` ? This is by design.
Regarding 2, 3, this is a known issue, this is due to whether we should allow duplicate note name under folder. I plan to discuss it in community, will fix it in follow up PR.
Regarding 6,7, I plan to fix it in follow up PR as they are not trivial to fix.
---
[GitHub] zeppelin issue #3163: ZEPPELIN-2619. Save note in ${notename_noteid}.zpln in...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/3163
Thanks all for reviewing and testing, will merge it soon
---
[GitHub] zeppelin issue #3163: ZEPPELIN-2619. Save note in ${notename_noteid}.zpln in...
Posted by mebelousov <gi...@git.apache.org>.
Github user mebelousov commented on the issue:
https://github.com/apache/zeppelin/pull/3163
@zjffdu
* Now usual note has tooltip "Move this note to trash" on trash button. Trashed note has tooltip "Remove this note permanently".
* Trashed note must be deleted on "Trash" button click.
* I know that noteId is unique. But what if somebody create "zpln" file in "notebook" folder and duplicate ID?
---
[GitHub] zeppelin issue #3163: ZEPPELIN-2619. Save note in ${notename_noteid}.zpln in...
Posted by mebelousov <gi...@git.apache.org>.
Github user mebelousov commented on the issue:
https://github.com/apache/zeppelin/pull/3163
@zjffdu
- Need change the behavior for button "Move note to Trash" for notes in Trash:
- * Need show another tooltip
- * And delete note, but not add another "~Trash" to note path
- Please add check for doubles of note ID. It's good to add warning to log about second note with the same ID.
I will continue testing. Please wait my approve :smiley:
---
[GitHub] zeppelin issue #3163: ZEPPELIN-2619. Save note in ${notename_noteid}.zpln in...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/3163
@mebelousov The issues you mentioned are resolved except the tooltip. I think the tooltip is correct, this is what zeppelin right now, first move note to trash, then user can remove it from trash permanently.
---
[GitHub] zeppelin issue #3163: ZEPPELIN-2619. Save note in [Title].zpln instead of [N...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/3163
@jongyoul @felixcheung CI is passed, could you help review it ?
---
[GitHub] zeppelin issue #3163: ZEPPELIN-2619. Save note in [Title].zpln instead of [N...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/3163
@jongyoul You can check the PR description for more details. Let me know what else you need to know.
@mebelousov The issue is fixed.
---
[GitHub] zeppelin issue #3163: ZEPPELIN-2619. Save note in ${notename_noteid}.zpln in...
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the issue:
https://github.com/apache/zeppelin/pull/3163
Wow!! Thank you guys for all your efforts @zjffdu @mebelousov
---
[GitHub] zeppelin issue #3163: ZEPPELIN-2619. Save note in ${notename_noteid}.zpln in...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/3163
Thanks @jongyoul I created ZEPPELIN-3758 for that, will do it after this PR is merged.
---
[GitHub] zeppelin issue #3163: ZEPPELIN-2619. Save note in ${notename_noteid}.zpln in...
Posted by mebelousov <gi...@git.apache.org>.
Github user mebelousov commented on the issue:
https://github.com/apache/zeppelin/pull/3163
@zjffdu
- In note tree plus sing near folder must create note in this folder not at root.
- It's seems double dots as folder name is still not fixed.
- Can we avoid CR/LF chars as names? Notes are created, but have "?" in file system name.
- Need to limit the name length for 255 symbols. For long names user get (tested on Ubuntu 16.04)
`Failed to create note.`
`IOException: Fail to create Note`
```python
import requests
import json
zeppelin_user, zeppelin_password = 'user1' , ''
zeppelin_host, zeppelin_port = 'localhost', 8080
class Izpepelin:
def __init__(self):
self.session = requests.Session()
self.session.auth = (zeppelin_user, zeppelin_password)
self.base_url = 'http://' + zeppelin_host + ':' + str(zeppelin_port)
def create_note(self, note_name):
body = {"name": note_name}
return self.session.post(self.base_url + '/api/notebook/', json = body).json()
super_names = ['Folder1/../double dot' '''Folder1/New
line''', 'Folder1/1111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111']
for name in super_names:
zepp.create_note(name)
```
---
[GitHub] zeppelin issue #3163: ZEPPELIN-2619. Save note in ${notename_noteid}.zpln in...
Posted by mebelousov <gi...@git.apache.org>.
Github user mebelousov commented on the issue:
https://github.com/apache/zeppelin/pull/3163
@zjffdu
- note can't be cloned
- need to deny folder name as ".." (double dot), because of after restart note will be in another folder
- note with only one empty paragraph doesn't have any paragraph after Zeppelin reload.
---
[GitHub] zeppelin issue #3163: ZEPPELIN-2619. Save note in ${notename_noteid}.zpln in...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/3163
@jongyoul @felixcheung I will merge it if no more comments, so that I can continue the follow up improvement.
---
[GitHub] zeppelin issue #3163: ZEPPELIN-2619. Save note in ${notename_noteid}.zpln in...
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the issue:
https://github.com/apache/zeppelin/pull/3163
@zjffdu Basically, I agree with this idea. BTW, do you have any plan to migrate from old format to new format?
---
[GitHub] zeppelin issue #3163: ZEPPELIN-2619. Save note in ${notename_noteid}.zpln in...
Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu commented on the issue:
https://github.com/apache/zeppelin/pull/3163
Thanks @mebelousov for testing.
* Need show another tooltip
Sorry I don't get it, what another tooltip do you mean ?
* And delete note, but not add another "~Trash" to note path
Do you mean to delete note permanently instead of moving to Trash ?
* noteId is unique, if there's duplicates, then it should fail to create new note.
---
[GitHub] zeppelin issue #3163: ZEPPELIN-2619. Save note in ${notename_noteid}.zpln in...
Posted by mebelousov <gi...@git.apache.org>.
Github user mebelousov commented on the issue:
https://github.com/apache/zeppelin/pull/3163
@zjffdu
No, I didn't see the popup with error explanation for user.
I have tested a lot with Russian symbols. Seems that Zeppelin works fine.
Thank you for this improvement. PR could be merged.
---
[GitHub] zeppelin issue #3163: ZEPPELIN-2619. Save note in ${notename_noteid}.zpln in...
Posted by mebelousov <gi...@git.apache.org>.
Github user mebelousov commented on the issue:
https://github.com/apache/zeppelin/pull/3163
@zjffdu, I'm back
1. I get IOException if folder name equals to "..". The same need for name "." (single dot).
1. Try to create 2 notes with the same path and name, then press "Move folder to Trash". On disk folder will be moved to trash, but note tree will contain bronen note link.
1. Still Trash button just renames note if it already in Trash. See first GIF.
1. In branch-0.8 only owners can rename note or change path, in your branch writers can change path and can trash note. (GIF2)
1. Note search (note-name-query form on main page) is broken. (GIF2)
1. Button "Clone this note" clones note to root folder. I expect cloning to the same folder. (GIF2)
---