You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by MikeTYChen <gi...@git.apache.org> on 2015/11/05 21:44:21 UTC
[GitHub] incubator-zeppelin pull request: ZEPPELIN-388: auto nav to new not...
GitHub user MikeTYChen opened a pull request:
https://github.com/apache/incubator-zeppelin/pull/394
ZEPPELIN-388: auto nav to new note
This PR relates to: https://issues.apache.org/jira/browse/ZEPPELIN-388 Auto navigate to the newly created Note right after its creation.
Current User Experience of Adding New Note (No response after creating a new note):
After Pull Request (moves to new note):
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/MikeTYChen/incubator-zeppelin ZEPPELIN-388
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-zeppelin/pull/394.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 #394
----
commit 4b7532bbb17603fbe2b6366d858d0d0fa388edcc
Author: Michael Chen <mi...@gmail.com>
Date: 2015-11-05T20:28:40Z
ZEPPELIN-388: auto nav to new note
----
---
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] incubator-zeppelin pull request: ZEPPELIN-388: auto nav to new not...
Posted by minahlee <gi...@git.apache.org>.
Github user minahlee commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/394#issuecomment-156015874
@MikeTYChen This issue has been resolved by #309 Do you mind to close this one?
---
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] incubator-zeppelin pull request: ZEPPELIN-388: auto nav to new not...
Posted by MikeTYChen <gi...@git.apache.org>.
Github user MikeTYChen commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/394#issuecomment-154518507
Thanks @r-kamath for great suggestion. I pushed a fix to add broadcast and remove the previous PR. Suggestions or improvements on the new 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] incubator-zeppelin pull request: ZEPPELIN-388: auto nav to new not...
Posted by corneadoug <gi...@git.apache.org>.
Github user corneadoug commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/394#issuecomment-154723900
It may even be easier to use a REST API than websocket on that one
---
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] incubator-zeppelin pull request: ZEPPELIN-388: auto nav to new not...
Posted by MikeTYChen <gi...@git.apache.org>.
Github user MikeTYChen closed the pull request at:
https://github.com/apache/incubator-zeppelin/pull/394
---
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] incubator-zeppelin pull request: ZEPPELIN-388: auto nav to new not...
Posted by r-kamath <gi...@git.apache.org>.
Github user r-kamath commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/394#issuecomment-154417725
@MikeTYChen I think the `broadcastNote` method in NotebookServer.java isn't sending the new notebook object to the current socket. May be we need to fix it and `getNewNoteID` should use the id from that response instead of picking it from the list. Thank you.
---
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] incubator-zeppelin pull request: ZEPPELIN-388: auto nav to new not...
Posted by minahlee <gi...@git.apache.org>.
Github user minahlee commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/394#issuecomment-154723437
@Leemoonsoo @MikeTYChen I pushed #309 earlier to address same issue as this one and it has similar approach @Leemoonsoo suggested above. Can you guys review 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] incubator-zeppelin pull request: ZEPPELIN-388: auto nav to new not...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/394#issuecomment-154254444
Thanks @MikeTYChen for really useful improvement.
CI build fails on
```
-------------------------------------------------------
T E S T S
-------------------------------------------------------
Running org.apache.zeppelin.ZeppelinIT
Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 7.275 sec <<< FAILURE! - in org.apache.zeppelin.ZeppelinIT
testAngularDisplay(org.apache.zeppelin.ZeppelinIT) Time elapsed: 7.18 sec <<< FAILURE!
java.lang.AssertionError: expected:<1> but was:<0>
at org.junit.Assert.fail(Assert.java:88)
at org.junit.Assert.failNotEquals(Assert.java:743)
at org.junit.Assert.assertEquals(Assert.java:118)
at org.junit.Assert.assertEquals(Assert.java:555)
at org.junit.Assert.assertEquals(Assert.java:542)
at org.apache.zeppelin.ZeppelinIT.createNewNoteAndGetName(ZeppelinIT.java:329)
at org.apache.zeppelin.ZeppelinIT.testAngularDisplay(ZeppelinIT.java:174)
Results :
Failed tests:
ZeppelinIT.testAngularDisplay:174->createNewNoteAndGetName:329 expected:<1> but was:<0>
Tests run: 1, Failures: 1, Errors: 0, Skipped: 0
```
It's because of test case get link of newly created notebook and click to move it (see https://github.com/apache/incubator-zeppelin/blob/master/zeppelin-server/src/test/java/org/apache/zeppelin/ZeppelinIT.java#L175). With this PR, ZeppelinIT test should be changed.
---
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] incubator-zeppelin pull request: ZEPPELIN-388: auto nav to new not...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/394#issuecomment-154722058
How about this approach?
[webapp]
* Open notebook on NOTE message. if notebook is not already opened.
[zeppelin server]
* when it receive NEW_NOTE from webapp, it send back NOTE (to make webapp open new notebook) as well as broadcast NOTES_INFO (to update notebook list)
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.
---
[GitHub] incubator-zeppelin pull request: ZEPPELIN-388: auto nav to new not...
Posted by vinayshukla <gi...@git.apache.org>.
Github user vinayshukla commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/394#issuecomment-154198794
Great work, thanks @MikeTYChen
---
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.
---