You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by eranwitkon <gi...@git.apache.org> on 2015/08/21 23:17:22 UTC
[GitHub] incubator-zeppelin pull request: Fix getRegisteredInterpreterList ...
GitHub user eranwitkon opened a pull request:
https://github.com/apache/incubator-zeppelin/pull/242
Fix getRegisteredInterpreterList to avoid adding null to the list
This PR fix the getRegisteredInterpreterList
InterpreterFactory is looping through the interpreters class name list from the config and registering an interpreter for every interpreter found on the disk.
when getRegisteredInterpreterList is called, for every item in the config-interpreters-class-name-list an item is added to the list, a RegisteredInterpreter if found or NULL.
This PR fix it by adding to the list only of the interpreter was found.
This also add a test to test the list are equal
Ready for review.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/eranwitkon/incubator-zeppelin 254
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-zeppelin/pull/242.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 #242
----
commit da0717f398828084dc9a99f9b398ec6d2217713a
Author: eranwitkon <go...@gmail.com>
Date: 2015-08-21T20:20:36Z
Fix getRegisteredInterpreterList,
avoid adding null to the list when class name from config is not registered
(only interpreters available on the disk (has interpreter folder) are registered.
findRegisteredInterpreterByClassName return null when class name not found.
(cherry picked from commit 6724e54)
----
---
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: Fix getRegisteredInterpreterList ...
Posted by eranwitkon <gi...@git.apache.org>.
Github user eranwitkon commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/242#issuecomment-134949085
This PR fix a bug in the getRegisteredInterpreterList.
Ready to review and merge, please.
---
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: Fix getRegisteredInterpreterList ...
Posted by eranwitkon <gi...@git.apache.org>.
Github user eranwitkon commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/242#issuecomment-133584373
I noticed this PR failed on CI. based on the log the InterpreterFactory JUnit failed.
Not sure what to do with it as the unit test pass locally
Please advise...
---
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: Fix getRegisteredInterpreterList ...
Posted by eranwitkon <gi...@git.apache.org>.
Github user eranwitkon closed the pull request at:
https://github.com/apache/incubator-zeppelin/pull/242
---
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: Fix getRegisteredInterpreterList ...
Posted by eranwitkon <gi...@git.apache.org>.
Github user eranwitkon commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/242#issuecomment-135522508
@Leemoonsoo I have PR #242 and PR #254 both opened.
#242 is the fix for the InterpreterFactory I will close it.
#254 is new REST API test - can this be reviewed?
---
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: Fix getRegisteredInterpreterList ...
Posted by eranwitkon <gi...@git.apache.org>.
Github user eranwitkon commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/242#issuecomment-135295501
Thus is not different, 240 is closed due to a problem I had with push.
This replace 240.
Thanks
בתאריך יום ד׳, 26 באוג׳ 2015, 20:13 מאת Lee moon soo <
notifications@github.com>:
> How it is different from #240
> <https://github.com/apache/incubator-zeppelin/pull/240> ?
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/incubator-zeppelin/pull/242#issuecomment-135111413>
> .
>
---
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: Fix getRegisteredInterpreterList ...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/242#issuecomment-135509696
My apologies, i mean with #241 (which is merged). Isn't it duplicated?
---
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: Fix getRegisteredInterpreterList ...
Posted by eranwitkon <gi...@git.apache.org>.
Github user eranwitkon commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/242#issuecomment-135517008
Sorry for that, I had 241 based on 242. Since I did not know if you are
going to accept both.
If #241 is merged then the fix is already in.
Thank.
I will close this pr as well.
בתאריך יום ה׳, 27 באוג׳ 2015, 21:08 מאת Lee moon soo <
notifications@github.com>:
> My apologies, i mean with #241
> <https://github.com/apache/incubator-zeppelin/pull/241> (which is
> merged). Isn't it duplicated?
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/incubator-zeppelin/pull/242#issuecomment-135509696>
> .
>
---
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: Fix getRegisteredInterpreterList ...
Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/242#issuecomment-135111413
How it is different from https://github.com/apache/incubator-zeppelin/pull/240 ?
---
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: Fix getRegisteredInterpreterList ...
Posted by eranwitkon <gi...@git.apache.org>.
Github user eranwitkon commented on the pull request:
https://github.com/apache/incubator-zeppelin/pull/242#issuecomment-134504419
Ready for review\merge
---
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.
---