You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by granturing <gi...@git.apache.org> on 2016/02/05 04:11:13 UTC
[GitHub] incubator-zeppelin pull request: ZEPPELIN-656 - Add support for us...
GitHub user granturing opened a pull request:
https://github.com/apache/incubator-zeppelin/pull/697
ZEPPELIN-656 - Add support for using Azure storage
### What is this PR for?
This is to allow the usage of Azure for notebook storage.
### What type of PR is it?
Improvement
### Todos
N/A
### Is there a relevant Jira issue?
[ZEPPELIN-656](https://issues.apache.org/jira/browse/ZEPPELIN-656)
### How should this be tested?
A full integration test will require an Azure storage account. I could provide an account offline if necessary unless someone already has one. I based this off the S3 storage repo. I did not see any integration tests for the S3 storage repo, but let me know if I need to add some for this.
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No, updates were made to zeppelin-site.xml.template explaining config settings
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/granturing/incubator-zeppelin azure-storage-backend
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-zeppelin/pull/697.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 #697
----
commit ce61f0e9173a7088147ebbeb9c178ccdb51f1270
Author: Silvio Fiorito <si...@granturing.com>
Date: 2016-02-05T03:04:05Z
Add support for using Azure storage
----
---
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-656 - Add support for us...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/697#issuecomment-180200911
Thanks for the new notebook storage implementation.
Could you also add configurations to https://github.com/apache/incubator-zeppelin/blob/master/docs/install/install.md#zeppelin-configuration ?
And i don't think missing integration tests for this contribution will be a blocker.
But any idea for the integration test?
---
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-656 - Add support for us...
Posted by granturing <gi...@git.apache.org>.
Github user granturing commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/697#issuecomment-181462386
Please see these latest commits. First, is to make changes as previously discussed for documenting optional "user" parameter as well as properly handling Azure exceptions.
Also, because the connection string has credentials I updated the ConfigurationsRestApi and NotebookServer to prevent it from being exposed through the API.
---
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-656 - Add support for us...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/697#issuecomment-185387181
LGTM and merge if there're no more discussions.
Thanks @granturing for great work!
---
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-656 - Add support for us...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/697#issuecomment-186004125
@granturing
I should aware of it before i merge, but i missed. I have created hotfix https://github.com/apache/incubator-zeppelin/pull/726 please review.
---
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-656 - Add support for us...
Posted by granturing <gi...@git.apache.org>.
Github user granturing closed the pull request at:
https://github.com/apache/incubator-zeppelin/pull/697
---
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-656 - Add support for us...
Posted by granturing <gi...@git.apache.org>.
Github user granturing commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/697#issuecomment-186001583
Yes, looks like a new checkpoint method was added to NotebookRepo interface. Thanks @Leemoonsoo let me know if there's anything I can do to help.
---
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-656 - Add support for us...
Posted by granturing <gi...@git.apache.org>.
Github user granturing closed the pull request at:
https://github.com/apache/incubator-zeppelin/pull/697
---
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-656 - Add support for us...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/697#issuecomment-184409000
any more comment?
---
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-656 - Add support for us...
Posted by minahlee <gi...@git.apache.org>.
Github user minahlee commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/697#issuecomment-180209575
Can you also update `zeppelin-distribution/src/bin_license/LICENSE` file for the newly added dependencies?
---
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-656 - Add support for us...
Posted by corneadoug <gi...@git.apache.org>.
Github user corneadoug commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/697#issuecomment-181642843
@granturing you can re-run CI by closing and reopening the 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-656 - Add support for us...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-zeppelin/pull/697
---
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-656 - Add support for us...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/697#issuecomment-186001113
Let me quickly make hotfix
---
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-656 - Add support for us...
Posted by granturing <gi...@git.apache.org>.
GitHub user granturing reopened a pull request:
https://github.com/apache/incubator-zeppelin/pull/697
ZEPPELIN-656 - Add support for using Azure storage
### What is this PR for?
This is to allow the usage of Azure for notebook storage.
### What type of PR is it?
Improvement
### Todos
N/A
### Is there a relevant Jira issue?
[ZEPPELIN-656](https://issues.apache.org/jira/browse/ZEPPELIN-656)
### How should this be tested?
A full integration test will require an Azure storage account. I could provide an account offline if necessary unless someone already has one. I based this off the S3 storage repo. I did not see any integration tests for the S3 storage repo, but let me know if I need to add some for this.
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update? Yes, license added
* Is there breaking changes for older versions? No
* Does this needs documentation? Yes, updates were made to zeppelin-site.xml.template explaining config settings as well as in the install doc
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/granturing/incubator-zeppelin azure-storage-backend
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-zeppelin/pull/697.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 #697
----
commit ce61f0e9173a7088147ebbeb9c178ccdb51f1270
Author: Silvio Fiorito <si...@granturing.com>
Date: 2016-02-05T03:04:05Z
Add support for using Azure storage
commit 928c8b55ea07b3312ba253b777e073dfec0f2ced
Author: Silvio Fiorito <si...@granturing.com>
Date: 2016-02-05T22:56:26Z
Added license info and docs for Azure storage settings, also added Azure-specific setting for user base path
commit cbfbe4d476a0b8ccce1b78f3be43480ca6afb11c
Author: Silvio Fiorito <si...@granturing.com>
Date: 2016-02-08T15:55:46Z
Clarified optional setting for Azure user folder and fixed Azure error handling
commit 06914727f7769ad253e44fb2bd73cdc718a8603b
Author: Silvio Fiorito <si...@granturing.com>
Date: 2016-02-08T16:36:20Z
Don't expose Azure storage connection string in UI since it contains credentials
commit 659e7f36e71563cc0fa70ce4e7d1de9753847e57
Author: Silvio Fiorito <si...@granturing.com>
Date: 2016-02-08T16:49:21Z
Additional locations where we have to block Azure credentials from being exposed
commit 433bdc002583881f7341dff533c673b9641c8994
Author: Silvio Fiorito <si...@granturing.com>
Date: 2016-02-11T03:43:49Z
Fix typo in enum name for user and share
----
---
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-656 - Add support for us...
Posted by granturing <gi...@git.apache.org>.
Github user granturing commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/697#issuecomment-181636357
Looks like the build failed due to some intermittent NPM errors? Can we re-run these?
---
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-656 - Add support for us...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/697#issuecomment-185996538
it looks like this might have broken the build?
```
[ERROR] COMPILATION ERROR :
[INFO] -------------------------------------------------------------
[ERROR] /home/travis/build/apache/incubator-zeppelin/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/AzureNotebookRepo.java:[43,8] org.apache.zeppelin.notebook.repo.AzureNotebookRepo is not abstract and does not override abstract method checkpoint(java.lang.String,java.lang.String) in org.apache.zeppelin.notebook.repo.NotebookRepo
```
https://travis-ci.org/apache/incubator-zeppelin/builds/110268133
---
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-656 - Add support for us...
Posted by khalidhuseynov <gi...@git.apache.org>.
Github user khalidhuseynov commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/697#issuecomment-184439606
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-656 - Add support for us...
Posted by granturing <gi...@git.apache.org>.
GitHub user granturing reopened a pull request:
https://github.com/apache/incubator-zeppelin/pull/697
ZEPPELIN-656 - Add support for using Azure storage
### What is this PR for?
This is to allow the usage of Azure for notebook storage.
### What type of PR is it?
Improvement
### Todos
N/A
### Is there a relevant Jira issue?
[ZEPPELIN-656](https://issues.apache.org/jira/browse/ZEPPELIN-656)
### How should this be tested?
A full integration test will require an Azure storage account. I could provide an account offline if necessary unless someone already has one. I based this off the S3 storage repo. I did not see any integration tests for the S3 storage repo, but let me know if I need to add some for this.
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update? Yes, license added
* Is there breaking changes for older versions? No
* Does this needs documentation? Yes, updates were made to zeppelin-site.xml.template explaining config settings as well as in the install doc
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/granturing/incubator-zeppelin azure-storage-backend
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-zeppelin/pull/697.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 #697
----
commit ce61f0e9173a7088147ebbeb9c178ccdb51f1270
Author: Silvio Fiorito <si...@granturing.com>
Date: 2016-02-05T03:04:05Z
Add support for using Azure storage
commit 928c8b55ea07b3312ba253b777e073dfec0f2ced
Author: Silvio Fiorito <si...@granturing.com>
Date: 2016-02-05T22:56:26Z
Added license info and docs for Azure storage settings, also added Azure-specific setting for user base path
commit cbfbe4d476a0b8ccce1b78f3be43480ca6afb11c
Author: Silvio Fiorito <si...@granturing.com>
Date: 2016-02-08T15:55:46Z
Clarified optional setting for Azure user folder and fixed Azure error handling
commit 06914727f7769ad253e44fb2bd73cdc718a8603b
Author: Silvio Fiorito <si...@granturing.com>
Date: 2016-02-08T16:36:20Z
Don't expose Azure storage connection string in UI since it contains credentials
commit 659e7f36e71563cc0fa70ce4e7d1de9753847e57
Author: Silvio Fiorito <si...@granturing.com>
Date: 2016-02-08T16:49:21Z
Additional locations where we have to block Azure credentials from being exposed
----
---
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-656 - Add support for us...
Posted by granturing <gi...@git.apache.org>.
Github user granturing commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/697#issuecomment-180645409
OK, I went ahead and added the license info for the Azure storage library as well as the docs. The S3 repo has a "user" folder setting, so I created a similar setting for Azure.
Let me know if you need anything else.
---
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.
---