[GitHub] zeppelin issue #1304: [ZEPPELIN-1190] [WIP] Visit Notebook Revision
Github user khalidhuseynov commented on the issue: https://github.com/apache/zeppelin/pull/1304 I think this can be closed --- 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 #1304: [ZEPPELIN-1190] [WIP] Visit Notebook Revision
Github user khalidhuseynov commented on the issue: https://github.com/apache/zeppelin/pull/1304 this have been stale for quite a while, so I'll start looking into 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 #1304: [ZEPPELIN-1190] [WIP] Visit Notebook Revision
Github user khalidhuseynov commented on the issue: https://github.com/apache/zeppelin/pull/1304 @corneadoug I've just tried the scenario you have, and it was running correctly (without np) when you switch to different revisions. the only thing is that it was running the `head` version which is expected behaviour. so could you check it out again? possibly that np error was due to different problem. --- 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 #1304: [ZEPPELIN-1190] [WIP] Visit Notebook Revision
Github user khalidhuseynov commented on the issue: https://github.com/apache/zeppelin/pull/1304 @corneadoug thanks for letting me know, i'll look into that and get back to 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] zeppelin issue #1304: [ZEPPELIN-1190] [WIP] Visit Notebook Revision
Github user corneadoug commented on the issue: https://github.com/apache/zeppelin/pull/1304 @khalidhuseynov I found out that websocket events at Notebook level are already blocked in the revision (I don't receive anything, and my actions are not affecting the original note). However there is some errors on Zeppelin server: https://cloud.githubusercontent.com/assets/710411/18079516/2ddd7944-6ecc-11e6-9be3-632dc0a1a704.png;> To reproduce, you can visit a revision form the dropdown. Then try running a paragraph (for example), this happen for all the Notebook websocket event (Run, Save etc...) The backend seems to be using a NotebookSocket param that isn't initiated in our case (which is good). However since the function are not checking the parameter, it generates a NPE --- 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 #1304: [ZEPPELIN-1190] [WIP] Visit Notebook Revision
Github user khalidhuseynov commented on the issue: https://github.com/apache/zeppelin/pull/1304 @bzz exactly, that's the point i was trying to make in #1254. @anthonycorbacho i'm completely agree with you, and that was the original starting premise. I'm happy that everyone got the point now! I'll reopen and rebase #1254 then. --- 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 #1304: [ZEPPELIN-1190] [WIP] Visit Notebook Revision
Github user bzz commented on the issue: https://github.com/apache/zeppelin/pull/1304 @corneadoug thank you for explaining! So we are talking about WS API to fetch full notebook state from the history by it's revision? Wich on client-side looks like [websocketMsgSrv.getNoteRevision($routeParams.noteId, $routeParams.revisionId);](https://github.com/apache/zeppelin/blob/3bd94ddc77ef9f9853ae978bf024d206378f16ba/zeppelin-web/src/components/websocketEvents/websocketMsg.service.js#L175) And on the back-end is a [NOTE_REVISION](https://github.com/apache/zeppelin/blob/36a7e38ffd6af614ad770a6e23ec1fd98a90c809/zeppelin-server/src/main/java/org/apache/zeppelin/socket/NotebookServer.java#L239) event in `NotebookServer`. @khalidhuseynov I think first of all, there is a bit of confusion here: this event is handled by `getNoteRevision` which looks more like `getNoteByRevision` because `Revision`, as discussed before, just represents the reference to the point in time, knowing which a full Note can be obtained, as it was back then. So `getNoteRevision` may be interpreted as a getter for Revision, rather than the Note. Then, it makes perfect sense to change the [Notebook.getNoteRevision()](https://github.com/apache/zeppelin/blob/5cdc02d3651fc8f5b7f9aa81f1d617007932b666/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/Notebook.java#L366) API, both, function name AND a signature, as that is the place where actual work of retrieving the Note is done, to something like: ```java getNoteByRevisionId(String noteId, String revisionId, AuthenticationInfo subject) ``` And this necessity, that comes from API client, at least for me, is an overriding argument in our discussion: you were right and we should adjust [NotebookRepo.get](https://github.com/apache/zeppelin/pull/1254/files#diff-eea417ea15b9480afaf52ebe1f51449dR92) interface and follow your design from #1254 (and close #1277). 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 #1304: [ZEPPELIN-1190] [WIP] Visit Notebook Revision
Github user anthonycorbacho commented on the issue: https://github.com/apache/zeppelin/pull/1304 @khalidhuseynov are we really sure about sending `complex object` as a parameter in the URL? How will it be represented? encoded? IMO it is not a good idea, if you ask for a resource, you need to call it by his Unique Identifier (_A unique identifier (UID) is a numeric or alphanumeric string that is associated with a single entity within a given system. IDs make it possible to address that entity, so that it can be accessed and interacted with_). I never saw in my hole life a complex object as ID, but i can be wrong (i dont use hipster api) but to me it looks like it is not the way to go. I would rather have a simple String/Int as a ID than a weird object, like we have for notebook, you call it with is Id eg: `/note/2A`. --- 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 #1304: [ZEPPELIN-1190] [WIP] Visit Notebook Revision
Github user corneadoug commented on the issue:
https://github.com/apache/zeppelin/pull/1304
@bzz
The front-end needs to be able to get the Notebook's revision and show it.
To do that, there is a websocket event that originally looked like that:
`websocketMsgSrv.getNoteRevision($routeParams.noteId, $routeParams.revisionId);`
After using it, it seemed that this websocket event was not well defined on
front-end side and that instead of `revisionId` I should be sending the whole
revision description object to the backend.
Which look like that:
```
revision: {
timestamp,
text,
revisionId
}
```
Now, unless there is a whole object to save, there should not be any reason
for the front-end to send a full object to the back-end so that it can
understand which revision I want. That's precisely what revisionId should be
for, especially if I want to have a custom URL that allows loading that
revision.
Furthermore, in order for me to send a full revision description object, it
would mean that I need to get the list of Revisions before I'm able to make any
call to load the Notebook revision.
---
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 #1304: [ZEPPELIN-1190] [WIP] Visit Notebook Revision
Github user bzz commented on the issue: https://github.com/apache/zeppelin/pull/1304 To avoid the confusion, could you guys quickly bring me up to speed with, what `API needs to be fixed to send Revision ID` means in this context? As far as I understood, we are talking in this PR about adding a new page on the frontend under URL `/notebook/:noteId/revision/:revisionId` and in order to be able to do that, there are some some expectations about REST API structure which are not met? Which REST API endpoint are we talking about and what exactly does not work now? --- 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 #1304: [ZEPPELIN-1190] [WIP] Visit Notebook Revision
Github user corneadoug commented on the issue: https://github.com/apache/zeppelin/pull/1304 @khalidhuseynov that doesn't solve anything, I can't work with Revision, it needs to be RevisionId. If you visit a URL like: /notebook/:noteId/revision/:revisionId All you have is the revisitionId and not the other informations, which means I can't make a call to get the Notebook content unless I get the list of Revisions first. --- 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 #1304: [ZEPPELIN-1190] [WIP] Visit Notebook Revision
Github user khalidhuseynov commented on the issue: https://github.com/apache/zeppelin/pull/1304 @corneadoug thanks for taking care of this! #1308 is merged and instead of `RevisionId` just send `Revision`. rebase should solve the problems. --- 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. ---
