[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1630 Tested and it's working nicely! LGTM ð --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/1630 Merge to master if there're 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/1630 Great work @tae-jun. LGTM! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1630 I found out bugs by adding loggers and fixed them! CI is green and it seems to work perfectly. I think it's good to go now ð Please when you have some time. 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1630 I've tested more and it was fine most times. But occasionally it doesn't work well. I think it should be tested more! I don't know why for now but I will figure it out and ping when it's ready :) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1630 Thanks @Leemoonsoo! I appreciate your 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user Leemoonsoo commented on the issue: https://github.com/apache/zeppelin/pull/1630 Tested and working really nicely. Looks awesome to me! Thanks @tae-jun for the great contribution! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1630 @AhyoungRyu @soralee Yeah! CI is green! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user soralee commented on the issue: https://github.com/apache/zeppelin/pull/1630 @tae-jun It is very AWESOME ð and thanks you for the accept my suggestion. Let me check again after CI is green ð --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1630 @AhyoungRyu Always thanks for the 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1630 @soralee What a nice suggestion! @tae-jun I think it's cool!! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1630 @soralee Hi! I added the feature you suggested. What do you think? Is title or content of warning message appropriate? Thanks for the cool suggestion!  --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1630 @soralee Thanks for the review! ð That's a great idea! I will give some effect on the front-end and ping you again. 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user soralee commented on the issue: https://github.com/apache/zeppelin/pull/1630 @tae-jun This is Cool! and It works well! So, I suggest one thing. I think it would be nice to appear popup which is whether user will merge the folders or not before merging folders. In my case, It is familiar to me :) 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1630 @AhyoungRyu Yeah! CI is green ð Please review! 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1630 @tae-jun Great work! Take your time and feel free to ping me again when you're ready. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1630 CI failure is because of me. I will fix it :) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1630 @AhyoungRyu Sorry for my late response! But it's due to tests :) I reproduced the error you mentioned and you were right! Furthermore, chasing a reason of the error, I notice that there are more possibilities of error. So I wrote more codes and added more test cases. ## Folder merging test  ## Renaming a high depth folder test  I thought this new feature is simple to implement... but now I don't think so...^^; I tested a lot of times, but maybe there are more bugs. So please test this when you have some time. 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1630 @AhyoungRyu Fixed what you mentioned :) Always thanks for the 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1630 @AhyoungRyu Yeah!!! Finally!!! CI is green!!! It was my fault! I didn't import `Folder` at `zeppelin-server`. (Why was it green before...?) Please take a look! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1630 I guess CI fails because it only recompiles changed files. (since build time is too short)  It should be about 30min. Is there any way to compile the whole project again? I ran CI on [my own repository](https://travis-ci.org/tae-jun/zeppelin). It failed on some profiles because of Ignite interpreter. (what is relevance between Ignite interpreter and my PR? ð) But it passed on other profiles whereas every profile failed on this repository. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1630 @AhyoungRyu I fixed what you mentioned ð 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1630 @AhyoungRyu Thanks! I also think `notebook` is a little bit weird :) I merged your PR thanks. This change will affect other insufficient privileges error messages as well: * Insufficient privileges to `read` ~~notebook~~ * Insufficient privileges to `update` ~~notebook~~ * Insufficient privileges to `remove` ~~notebook~~ * Insufficient privileges to `write` ~~notebook~~ * Insufficient privileges to `clear output` ~~notebook~~ I prefer removing `notebook`. What do you think guys? If anyone has a concern about this, please tell. And about CI, I suffered from that error when I tried to compile only one project (e.g. `zeppelin-server`). Since `zeppelin-server` depends on `zeppelin-interpreter` and `zeppelin-zengine`, I had to build two dependencies and `install` them on local maven repo, which can be done with `mvn clean package install -DskipTests`. If not, when it tries to compile `zeppelin-server`, it cannot find symbols of `zeppelin-zengine` and goes to fail. However, I'm not sure that CI only recompiles changed files. I will look into it, and tell you if I get some more information. (CI was green before ð) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1630 @tae-jun And one more thing, it would be better to keep consistency between `Note` and `Notebook`(combination of several notes). I pushed one commit to your branch, could you please take a look https://github.com/tae-jun/zeppelin/pull/1? It will be shown when user rename the folder(I removed `notebook`): https://cloud.githubusercontent.com/assets/10060731/20461892/d857a00e-af0c-11e6-92de-095215062fdd.png;> And then user rename the note: https://cloud.githubusercontent.com/assets/10060731/20461895/efdaa4b0-af0c-11e6-904c-667f6bcc82cc.png;> And also when user create a note, I changed `Example: NoteDir1/Notebook1` -> `NoteDir1/Note1` https://cloud.githubusercontent.com/assets/10060731/20461899/04f7cd78-af0d-11e6-97a8-825b1fec1932.png;> --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1630 @tae-jun It works well when I tried to rename folder name. And I think It's good idea to let user know which notes that he don't have permission exactly. Thanks :) But currently CI is failed with below compilation error. Could you take a look again? ``` [ERROR] COMPILATION ERROR : [INFO] - [ERROR] /home/travis/build/apache/zeppelin/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java:[774,5] cannot find symbol symbol: class Folder location: class org.apache.zeppelin.socket.NotebookServer [INFO] 1 error ``` --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1630 @AhyoungRyu Thanks for your kind review. I appreciate it. Actually, I didn't test with Shiro ^^; and didn't know what `op` parameter means. So thanks very much. I changed `renameFolder` to `rename folder of` you addressed and also `renameNote` to `rename`. Now it looks like this: https://cloud.githubusercontent.com/assets/8201019/20454545/462de7b8-ae87-11e6-9dea-89a355e7cf30.png;> https://cloud.githubusercontent.com/assets/8201019/20454517/77115a1e-ae86-11e6-9284-baf39b3c5498.png;> I also included note name which needs permission ð 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1630 @tae-jun Build it again and tested, but It works well as you intended. It was my bad. I tested two cases: \w & \wt Shiro. Both are works well. Probably it's a nitpick, https://cloud.githubusercontent.com/assets/10060731/20440848/06789188-adc2-11e6-89cd-7ce9c2c20cb7.png;> when I try to rename the folder name at home screen, the above msg is shown up. Could you change `renameFolder Notebook` to `rename folder`? (I know `renameFolder` comes from `op` and `notebook` is hardcoded, but sounds a bit awkward ã _ã ) --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1630 @AhyoungRyu ping ping~ I tested!  But it just works fine with me. I tested both running ZeppelinServer from IntelliJ and binary built from `mvn clean package -DskipTests` I use macOS 10.12.1 and Chrome. Could you please share your test environment? ð --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1630 @tae-jun Sorry for my late response. I tested but can't edit the dir name in root dir level. The folder name can be modified only in below level. Please see the below attached gif img.  --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user AhyoungRyu commented on the issue: https://github.com/apache/zeppelin/pull/1630 Awesome. Let me test this. Will ping you 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1630: [ZEPPELIN-1629] Enable renaming folder from the main p...
Github user tae-jun commented on the issue: https://github.com/apache/zeppelin/pull/1630 Fixed error and CI is green! Ready for 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
