You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by dirceusemighini <gi...@git.apache.org> on 2015/09/22 15:48:00 UTC
[GitHub] incubator-zeppelin pull request: Save all repos, and not just the ...
GitHub user dirceusemighini opened a pull request:
https://github.com/apache/incubator-zeppelin/pull/318
Save all repos, and not just the first two
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/dirceusemighini/incubator-zeppelin master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-zeppelin/pull/318.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 #318
----
commit 71a4bfb054d470b524dc328bfa36954b6b77a296
Author: Dirceu Semighini Filho <di...@gmail.com>
Date: 2015-09-22T13:47:30Z
Save all repos, and not just the first two
----
---
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: Save all repos, and not just the ...
Posted by dirceusemighini <gi...@git.apache.org>.
Github user dirceusemighini commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/318#issuecomment-146211856
Hi Guys, I was on vacation and just returned.
So, we still can't open more than two repos, but does this code will help
on saving when the option to open more than one is available, right?
2015-10-04 4:54 GMT-03:00 Khalid Huseynov <no...@github.com>:
> I think this PR doesn't add synchronization for more than 2 repos, it just
> saves to all available repos. The reason is that there's still limit on the
> number of repos which is 2 currently (
> https://github.com/apache/incubator-zeppelin/blob/master/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoSync.java#L42).
> So basically we still don't get more than 2 repo saves. On the other
> hand, it's good idea to be able to handle more than 2 repos, and it'll be
> coming soon probably
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/incubator-zeppelin/pull/318#issuecomment-145325073>
> .
>
---
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: Save all repos, and not just the ...
Posted by khalidhuseynov <gi...@git.apache.org>.
Github user khalidhuseynov commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/318#issuecomment-145325073
I think this PR doesn't add synchronization for more than 2 repos, it just saves to all available repos. The reason is that there's still limit on the number of repos which is 2 currently (https://github.com/apache/incubator-zeppelin/blob/master/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoSync.java#L42). So basically we still don't get more than 2 repo `saves`. On the other hand, it's good idea to be able to handle more than 2 repos, and it'll be coming soon probably
---
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: Save all repos, and not just the ...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/318#issuecomment-148064890
Thanks @dirceusemighini. Please take your time.
As @khalidhuseynov commented, I think [sync()](https://github.com/apache/incubator-zeppelin/blob/master/zeppelin-zengine/src/main/java/org/apache/zeppelin/notebook/repo/NotebookRepoSync.java#L159) also need to aware of more than 3 repos.
---
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: Save all repos, and not just the ...
Posted by khalidhuseynov <gi...@git.apache.org>.
Github user khalidhuseynov commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/318#issuecomment-145230159
@dirceusemighini I believe `getSimpleName` should be fine
---
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: Save all repos, and not just the ...
Posted by hsaputra <gi...@git.apache.org>.
Github user hsaputra commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/318#issuecomment-146314427
Hi @dirceusemighini, could you also update the description of this PR with more info of what is the intended change and the desired behavior?
It will help reviewers and others to look at it and in the future when we need to revisit old PRs.
---
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.
---
Re: [GitHub] incubator-zeppelin pull request: Save all repos, and not
just the ...
Posted by Dirceu Semighini Filho <di...@gmail.com>.
Hi Khaliduseynov,
Thank you for looking into this, is there any guide on how can I run CI
here?
2015-09-22 23:03 GMT-03:00 khalidhuseynov <gi...@git.apache.org>:
> Github user khalidhuseynov commented on the pull request:
>
>
> https://github.com/apache/incubator-zeppelin/pull/318#issuecomment-142471747
>
> Existing implementation gives more focus on the existence of maximum
> of two backend repos. The change makes it look more logical in case we add
> more backend repos. Looks good except need to address the following issues:
> - Fix `log` to reflect which repo index saving fails
> - Fix styling to pass CI
>
>
> ---
> 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: Save all repos, and not just the ...
Posted by khalidhuseynov <gi...@git.apache.org>.
Github user khalidhuseynov commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/318#issuecomment-142471747
Existing implementation gives more focus on the existence of maximum of two backend repos. The change makes it look more logical in case we add more backend repos. Looks good except need to address the following issues:
- Fix `log` to reflect which repo index saving fails
- Fix styling to pass CI
---
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: Save all repos, and not just the ...
Posted by dirceusemighini <gi...@git.apache.org>.
Github user dirceusemighini commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/318#issuecomment-142589013
I used getSimple name in the repo class, do you think that someone could use an anonymous class in here?
---
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: Save all repos, and not just the ...
Posted by dirceusemighini <gi...@git.apache.org>.
Github user dirceusemighini commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/318#issuecomment-148039317
Hi @Leemoonsoo latter this week I will update the test to assure that the code is running fine.
---
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: Save all repos, and not just the ...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/318#issuecomment-145210388
Nice improvement! Could you update https://github.com/apache/incubator-zeppelin/blob/master/zeppelin-zengine/src/test/java/org/apache/zeppelin/notebook/repo/NotebookRepoSyncTest.java to test with more than 3 notebook repo?
---
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.
---