Skip to content

Conversation

@khalidhuseynov
Copy link
Member

@khalidhuseynov khalidhuseynov commented Nov 24, 2016

What is this PR for?

This is to enable switching between revisions and being able to set Head/current note to one of those revisions. Currently notes are editable when switching between them, next step after this PR would be make them non-editable during revision switches.

What type of PR is it?

Improvement | Feature

Todos

  • - routes and switching revisions
  • - set revision button and api
  • - test for setting revision

What is the Jira issue?

ZEPPELIN-1190

How should this be tested?

set config in conf/zeppelin-env.sh

export ZEPPELIN_NOTEBOOK_STORAGE="org.apache.zeppelin.notebook.repo.GitNotebookRepo"

and switch between notes in note action bar, as well as set certain revisions as shown below

Screenshots (if appropriate)

switch revisions

Questions:

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

@khalidhuseynov khalidhuseynov reopened this Dec 2, 2016
@khalidhuseynov khalidhuseynov force-pushed the feat/switch-revisions branch 2 times, most recently from d5963a0 to 68c5d97 Compare December 2, 2016 10:11
@khalidhuseynov khalidhuseynov changed the title [ZEPPELIN-1190] [WIP] Visit and switch notebook revisions [ZEPPELIN-1190] Visit and switch notebook revisions Dec 2, 2016
@khalidhuseynov
Copy link
Member Author

this is ready for review. @Leemoonsoo let me know if you have any feedback regarding this one.

@Leemoonsoo
Copy link
Member

Tested and it works well. Great work @khalidhuseynov.

But when i visit revision, shouldn't it be immutable? i can still edit and run.

@khalidhuseynov
Copy link
Member Author

@Leemoonsoo yes right currently it's editable and runnable, which is updated in PR description. Actually I want to add additional offline or report-offline mode and basically switch to that mode when viewing revisions. I think that's quite separate issue from this one and can be addressed right after this one, what do you think?

@Leemoonsoo
Copy link
Member

@khalidhuseynov sounds good.

Let's wait for CI becomes green. Other than that, Looks good to me.

@khalidhuseynov
Copy link
Member Author

created issue under ZEPPELIN-1745 for second part

@khalidhuseynov khalidhuseynov reopened this Dec 2, 2016
@khalidhuseynov khalidhuseynov force-pushed the feat/switch-revisions branch 3 times, most recently from 01ed7b1 to a47bc06 Compare December 3, 2016 05:56
@khalidhuseynov
Copy link
Member Author

khalidhuseynov commented Dec 3, 2016

not sure why only one profile keeps failing with all rest api tests

@khalidhuseynov khalidhuseynov force-pushed the feat/switch-revisions branch 4 times, most recently from cba7306 to 25ccf8f Compare December 5, 2016 02:44
@khalidhuseynov khalidhuseynov reopened this Dec 5, 2016
@khalidhuseynov khalidhuseynov force-pushed the feat/switch-revisions branch 3 times, most recently from 6500bec to 506fea0 Compare December 6, 2016 02:11
@khalidhuseynov
Copy link
Member Author

rebased from master, and 6th profile keeps failing with Zeppelin shutting down because of no SPARK_HOME. So initially fails to untar spark package:

+mkdir -p .spark-dist
+cd .spark-dist
+[[ ! -f spark-1.4.1-bin-hadoop2.3.tgz ]]
+cp spark-1.4.1-bin-hadoop2.3.tgz ..
+cd ..
+tar zxf spark-1.4.1-bin-hadoop2.3.tgz

gzip: stdin: unexpected end of file
tar: Unexpected EOF in archive
tar: Unexpected EOF in archive
tar: Error is not recoverable: exiting now
+echo 'Unable to extract spark-1.4.1-bin-hadoop2.3.tgz'
Unable to extract spark-1.4.1-bin-hadoop2.3.tgz
+rm -rf spark-1.4.1-bin-hadoop2.3
+rm -f spark-1.4.1-bin-hadoop2.3.tgz
+set +xe

and then in tests:

02:22:06,298  INFO org.apache.zeppelin.rest.AbstractTestRestApi:181 - Test Zeppelin stared.
SPARK HOME detected null
02:22:06,302  INFO org.apache.zeppelin.rest.AbstractTestRestApi:298 - Terminating test Zeppelin...

which fails all rest api tests. obviously this is not related to this PR, although not sure why spark untar keeps failing in same profile. logs are here.

@khalidhuseynov khalidhuseynov force-pushed the feat/switch-revisions branch 4 times, most recently from 02c2a19 to ab7db29 Compare December 12, 2016 06:58
only when enabled when in certain revision
@khalidhuseynov
Copy link
Member Author

@Leemoonsoo CI is finally green, I think it's ready to go

@Leemoonsoo
Copy link
Member

LGTM and merge to master if there're no further discussions.
@khalidhuseynov Great work!

@asfgit asfgit closed this in 2fe5a00 Dec 15, 2016
@khalidhuseynov khalidhuseynov deleted the feat/switch-revisions branch December 15, 2016 03:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants