Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ZEPPELIN-1189] Get note revision websocket api #1192

Conversation

khalidhuseynov
Copy link
Contributor

What is this PR for?

Adds websocket api for getting note revision.

What type of PR is it?

Improvement | Feature

Todos

  • - add backend websocket handle
  • - add frontend call

What is the Jira issue?

#1189

How should this be tested?

green CI (can be tested once frontend implemented)

Screenshots (if appropriate)

Questions:

  • Does the licenses files need update? no
  • Is there breaking changes for older versions? no
  • Does this needs documentation? no

@khalidhuseynov khalidhuseynov changed the title Versioning/get note revision api [ZEPPELIN-1189] Get note revision websocket api Jul 15, 2016
@bzz
Copy link
Member

bzz commented Jul 17, 2016

Looks like CI fails, could you please followup on the reason?

One more question - it looks like this PR not only add websocket API but change backend API and modify every NotebookRrpo implementation.

Is there a way to avoid such changes in a single PR? Or is there a strong reason for breaking API here?

@khalidhuseynov
Copy link
Contributor Author

@bzz fixed the issue with CI.
regarding change in api, you're right. it's better not to change api as a part of different PR, so I reverted it back. The main point in changing Revision object into just String representing revisionId was that revisionId can completely represent Revision object. However we don't need to address it right now, and it can be addressed as an improvement with addressing pros and cons.

@bzz
Copy link
Member

bzz commented Jul 19, 2016

Sounds great, thank you for taking care!

CI is still failing though :\ Please feel free to followup the failure here and I will be happy to help, if needed!

@khalidhuseynov
Copy link
Contributor Author

@bzz addressed the changes and CI is green

@bzz
Copy link
Member

bzz commented Jul 20, 2016

Looks great to me! Thank you for taking care. Megin if there is no further discussion

@asfgit asfgit closed this in 67cb828 Jul 20, 2016
@khalidhuseynov khalidhuseynov deleted the versioning/get-note-revision-api branch July 20, 2016 03:14
PhilippGrulich pushed a commit to SWC-SENSE/zeppelin that referenced this pull request Aug 8, 2016
### What is this PR for?
Adds websocket api for getting note revision.

### What type of PR is it?
Improvement | Feature

### Todos
* [x] - add backend websocket handle
* [x] - add frontend call

### What is the Jira issue?
[apache#1189](https://issues.apache.org/jira/browse/ZEPPELIN-1189)

### How should this be tested?
green CI (can be tested once frontend implemented)

### Screenshots (if appropriate)

### Questions:
* Does the licenses files need update? no
* Is there breaking changes for older versions? no
* Does this needs documentation? no

Author: Khalid Huseynov <khalidhnv@nflabs.com>
Author: Khalid Huseynov <khalidhuseynov@Khalids-MacBook-Pro.local>
Author: Khalid Huseynov <khalidhnv@gmail.com>

Closes apache#1192 from khalidhuseynov/versioning/get-note-revision-api and squashes the following commits:

f1ab994 [Khalid Huseynov] Merge branch 'master' into versioning/get-note-revision-api
683b481 [Khalid Huseynov] receive Revision object from frontend
aa0a7d6 [Khalid Huseynov] Revert "change NotebookRepo api to get note revision from Revision object to String revId"
ce097ed [Khalid Huseynov] add throws to notebook getRevisionNote
baaa704 [Khalid Huseynov] change NotebookRepo api to get note revision from Revision object to String revId
d9751c3 [Khalid Huseynov] add backend ws api to get note revision
3783fc9 [Khalid Huseynov] receive ws NOTE_REVISION msg
79f8ac9 [Khalid Huseynov] add getNoteRevision to front
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants