You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by zjffdu <gi...@git.apache.org> on 2016/12/09 02:15:04 UTC

[GitHub] zeppelin pull request #1740: ZEPPELIN-1773. Remove method destroy() in Inter...

GitHub user zjffdu opened a pull request:

    https://github.com/apache/zeppelin/pull/1740

    ZEPPELIN-1773. Remove method destroy() in Interpreter

    ### What is this PR for?
    IMHO `destroy` is a little functional duplicated with `close`. And it seems currently only cassandra use destroy and it just calls `close()` https://github.com/apache/zeppelin/blob/master/cassandra/src/main/java/org/apache/zeppelin/cassandra/CassandraInterpreter.java#L231. So IMO, we could delete destroy method, also we can delete the destroy method in `InterpreterGroup`. 
    
    
    ### What type of PR is it?
    [Improvement]
    
    ### Todos
    * [ ] - Task
    
    ### What is the Jira issue?
    * https://issues.apache.org/jira/browse/ZEPPELIN-1773
    
    ### How should this be tested?
    Just remove something so the existing test pass should be sufficient. 
    
    ### 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/zjffdu/zeppelin ZEPPELIN-1773

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zeppelin/pull/1740.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 #1740
    
----
commit 02c3fbe61dad84180836b226d0e50b90958ae01b
Author: Jeff Zhang <zj...@apache.org>
Date:   2016-12-09T02:12:51Z

    ZEPPELIN-1773. Remove method destroy() in Interpreter

----


---
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] zeppelin pull request #1740: ZEPPELIN-1773. Remove method destroy() in Inter...

Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu closed the pull request at:

    https://github.com/apache/zeppelin/pull/1740


---
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] zeppelin pull request #1740: ZEPPELIN-1773. Remove method destroy() in Inter...

Posted by zjffdu <gi...@git.apache.org>.
GitHub user zjffdu reopened a pull request:

    https://github.com/apache/zeppelin/pull/1740

    ZEPPELIN-1773. Remove method destroy() in Interpreter

    ### What is this PR for?
    IMHO `destroy` is a little functional duplicated with `close`. And it seems currently only cassandra use destroy and it just calls `close()` https://github.com/apache/zeppelin/blob/master/cassandra/src/main/java/org/apache/zeppelin/cassandra/CassandraInterpreter.java#L231. So IMO, we could delete destroy method, also we can delete the destroy method in `InterpreterGroup`.  Besides, I move the terminate remoteInterpreterProcess in `close`.
    
    
    ### What type of PR is it?
    [Improvement]
    
    ### Todos
    * [ ] - Task
    
    ### What is the Jira issue?
    * https://issues.apache.org/jira/browse/ZEPPELIN-1773
    
    ### How should this be tested?
    Just remove something so the existing test pass should be sufficient. 
    
    ### 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/zjffdu/zeppelin ZEPPELIN-1773

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zeppelin/pull/1740.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 #1740
    
----
commit 4fd2e38db99c815da783eea4232a20fd9bd30178
Author: Jeff Zhang <zj...@apache.org>
Date:   2016-12-09T02:12:51Z

    ZEPPELIN-1773. Remove method destroy() in Interpreter

----


---
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] zeppelin pull request #1740: ZEPPELIN-1773. Remove method destroy() in Inter...

Posted by zjffdu <gi...@git.apache.org>.
GitHub user zjffdu reopened a pull request:

    https://github.com/apache/zeppelin/pull/1740

    ZEPPELIN-1773. Remove method destroy() in Interpreter

    ### What is this PR for?
    IMHO `destroy` is a little functional duplicated with `close`. And it seems currently only cassandra use destroy and it just calls `close()` https://github.com/apache/zeppelin/blob/master/cassandra/src/main/java/org/apache/zeppelin/cassandra/CassandraInterpreter.java#L231. So IMO, we could delete destroy method, also we can delete the destroy method in `InterpreterGroup`.  Besides, I move the terminate remoteInterpreterProcess in `close`.
    
    
    ### What type of PR is it?
    [Improvement]
    
    ### Todos
    * [ ] - Task
    
    ### What is the Jira issue?
    * https://issues.apache.org/jira/browse/ZEPPELIN-1773
    
    ### How should this be tested?
    Just remove something so the existing test pass should be sufficient. 
    
    ### 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/zjffdu/zeppelin ZEPPELIN-1773

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zeppelin/pull/1740.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 #1740
    
----
commit 4fd2e38db99c815da783eea4232a20fd9bd30178
Author: Jeff Zhang <zj...@apache.org>
Date:   2016-12-09T02:12:51Z

    ZEPPELIN-1773. Remove method destroy() in Interpreter

----


---
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] zeppelin issue #1740: ZEPPELIN-1773. Remove method destroy() in Interpreter

Posted by Leemoonsoo <gi...@git.apache.org>.
Github user Leemoonsoo commented on the issue:

    https://github.com/apache/zeppelin/pull/1740
  
    LGTM. CI failure looks irrelevant.
    Merge to master if there're no further discussions.


---
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] zeppelin pull request #1740: ZEPPELIN-1773. Remove method destroy() in Inter...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/zeppelin/pull/1740


---
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] zeppelin pull request #1740: ZEPPELIN-1773. Remove method destroy() in Inter...

Posted by zjffdu <gi...@git.apache.org>.
Github user zjffdu closed the pull request at:

    https://github.com/apache/zeppelin/pull/1740


---
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.
---