You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by khalidhuseynov <gi...@git.apache.org> on 2015/12/29 12:18:32 UTC
[GitHub] incubator-zeppelin pull request: ZEPPELIN-543: Storage initializat...
GitHub user khalidhuseynov opened a pull request:
https://github.com/apache/incubator-zeppelin/pull/582
ZEPPELIN-543: Storage initialization failure recovery
### What is this PR for?
Handling edge case failures when initializing notebook storage class(es).
### What type of PR is it?
Improvement
### Todos
* [x] - fix repo initialization
* [x] - add different config tests
### Is there a relevant Jira issue?
[ZEPPELIN-543](https://issues.apache.org/jira/browse/ZEPPELIN-543)
### How should this be tested?
There're various test cases (mostly included as unit tests).
One of the test scenarios:
1. in `conf/zeppelin-env.sh` add the following line
```
export ZEPPELIN_NOTEBOOK_STORAGE="org.apache.zeppelin.notebook.repo.VFSNotebookRepo,org.apache.zeppelin.notebook.repo.DummyNotebookRepo"
```
2. Start Zeppelin
3. Previously Zeppelin would fail to start, now you would see that only first storage class is initialized and second just didn't initialize.
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/khalidhuseynov/incubator-zeppelin storage-init-failure-recovery
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-zeppelin/pull/582.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 #582
----
commit d4dc62f85ebec0421dbd5763258de368e49882f5
Author: Khalid Huseynov <kh...@nflabs.com>
Date: 2015-12-29T10:31:13Z
remove throwing exception, handle gracefully
commit 8f766320aea2bad14d5aa9c8df97607213a4733c
Author: Khalid Huseynov <kh...@nflabs.com>
Date: 2015-12-29T10:31:52Z
add tests for different configs
----
---
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-543: Storage initializat...
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/582#issuecomment-167912938
+1 @HeartSaVioR. It's better to display the warning
---
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-543: Storage initializat...
Posted by khalidhuseynov <gi...@git.apache.org>.
Github user khalidhuseynov commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/582#issuecomment-168921896
@bzz thanks for the feedback! I updated naming for the methods I found starting with uppercase, let me know if there's anything else left. Otherwise it's good to go.
---
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-543: Storage initializat...
Posted by bzz <gi...@git.apache.org>.
Github user bzz commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/582#issuecomment-168935527
Thank you, looks great to me.
Merging if there is no more discussion.
---
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-543: Storage initializat...
Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/582#issuecomment-167817866
@khalidhuseynov I've tested what you describe and saw the warning messages in a log and tests cases look nice to explain what you want to fix. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-zeppelin pull request: ZEPPELIN-543: Storage initializat...
Posted by bzz <gi...@git.apache.org>.
Github user bzz commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/582#issuecomment-168875123
+1 for better user notifications\warnings in the separate PR.
@khalidhuseynov could you please check if the method naming follows the project code conventions.
Shall we update and merge this one after that, if there is no discussion?
---
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-543: Storage initializat...
Posted by khalidhuseynov <gi...@git.apache.org>.
Github user khalidhuseynov commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/582#issuecomment-167920418
@HeartSaVioR @jongyoul thanks for the feedback! Actually displaying warning to the user is a whole another issue that I came across few times recently, and I was thinking to address it soon. Definitely I'll create an issue and +1 for that.
---
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-543: Storage initializat...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/582#issuecomment-167901055
After this PR, some repositories can't be loaded but we can't see what repositories are loaded via Web UI.
I think it may be better to have page to Zeppelin Web UI and let page shows loaded repositories.
(It could be filed to another JIRA issue.)
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-543: Storage initializat...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-zeppelin/pull/582
---
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-543: Storage initializat...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/582#issuecomment-167900375
I left some minor comments but code and unit tests looks great. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---