[GitHub] zeppelin pull request #2397: [ZEPPELIN-2592] Ensure open stream is closed

classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin pull request #2397: [ZEPPELIN-2592] Ensure open stream is closed

prabhjyotsingh
GitHub user necosta opened a pull request:

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

    [ZEPPELIN-2592] Ensure open stream is closed

    ### What is this PR for?
    Fix for missing close statement on input streams found on Helium Bundle factory
   
    ### What type of PR is it?
    Bug Fix
   
    ### What is the Jira issue?
    * https://issues.apache.org/jira/browse/ZEPPELIN-2592
   
    ### How should this be tested?
    NA
   
    ### Screenshots (if appropriate)
   
    ### Questions:
    * Does the licenses files need update? N
    * Is there breaking changes for older versions? N
    * Does this needs documentation? N


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/nokia/zeppelin zeppelin2592

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

    https://github.com/apache/zeppelin/pull/2397.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 #2397
   
----
commit cfd64d230e055e6144d0eb0b98129bc3ce5d0066
Author: Nelson Costa <[hidden email]>
Date:   2017-06-06T15:20:02Z

    [ZEPPELIN-2592] Ensure open stream is closed

----


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2397: [ZEPPELIN-2592] Ensure open stream is closed

prabhjyotsingh
Github user necosta commented on the issue:

    https://github.com/apache/zeppelin/pull/2397
 
    I was hoping I had more time to refactor some other streams usage but, for now, just created this pull request for the missing close stream in Helium. Any questions, please let me know. Thanks.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin pull request #2397: [ZEPPELIN-2592] Ensure open stream is closed

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user necosta closed the pull request at:

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


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin pull request #2397: [ZEPPELIN-2592] Ensure open stream is closed

prabhjyotsingh
In reply to this post by prabhjyotsingh
GitHub user necosta reopened a pull request:

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

    [ZEPPELIN-2592] Ensure open stream is closed

    ### What is this PR for?
    Fix for missing close statement on input streams found on Helium Bundle factory
   
    ### What type of PR is it?
    Bug Fix
   
    ### What is the Jira issue?
    * https://issues.apache.org/jira/browse/ZEPPELIN-2592
   
    ### How should this be tested?
    NA
   
    ### Screenshots (if appropriate)
   
    ### Questions:
    * Does the licenses files need update? N
    * Is there breaking changes for older versions? N
    * Does this needs documentation? N


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/nokia/zeppelin zeppelin2592

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

    https://github.com/apache/zeppelin/pull/2397.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 #2397
   
----
commit f87c38a499a90ca2188f30b38777bba4bbad2ea6
Author: Nelson Costa <[hidden email]>
Date:   2017-06-06T15:35:06Z

    [ZEPPELIN-2592] Ensure open stream is closed

----


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2397: [ZEPPELIN-2592] Ensure open stream is closed

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user jongyoul commented on the issue:

    https://github.com/apache/zeppelin/pull/2397
 
    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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2397: [ZEPPELIN-2592] Ensure open stream is closed

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user andreaTP commented on the issue:

    https://github.com/apache/zeppelin/pull/2397
 
    to be 100% sure to close properly the stream the `close` method should be called within a `finally` statement of a `try`


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2397: [ZEPPELIN-2592] Ensure open stream is closed

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user jongyoul commented on the issue:

    https://github.com/apache/zeppelin/pull/2397
 
    @andreaTP  try() doesn't call close when it occurs exception?


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2397: [ZEPPELIN-2592] Ensure open stream is closed

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user andreaTP commented on the issue:

    https://github.com/apache/zeppelin/pull/2397
 
    `try-with-resources` is used only for `InputStream` here.
    there is nothing on outputstream.
    https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2397: [ZEPPELIN-2592] Ensure open stream is closed

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user jongyoul commented on the issue:

    https://github.com/apache/zeppelin/pull/2397
 
    @andreaTP Thanks for fixing it. BTW, I think we'd better use try-with-resources of outputStream


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin pull request #2397: [ZEPPELIN-2592] Ensure open stream is closed

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user necosta closed the pull request at:

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


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin pull request #2397: [ZEPPELIN-2592] Ensure open stream is closed

prabhjyotsingh
In reply to this post by prabhjyotsingh
GitHub user necosta reopened a pull request:

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

    [ZEPPELIN-2592] Ensure open stream is closed

    ### What is this PR for?
    Fix for missing close statement on input streams found on Helium Bundle factory
   
    ### What type of PR is it?
    Bug Fix
   
    ### What is the Jira issue?
    * https://issues.apache.org/jira/browse/ZEPPELIN-2592
   
    ### How should this be tested?
    NA
   
    ### Screenshots (if appropriate)
   
    ### Questions:
    * Does the licenses files need update? N
    * Is there breaking changes for older versions? N
    * Does this needs documentation? N


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/nokia/zeppelin zeppelin2592

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

    https://github.com/apache/zeppelin/pull/2397.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 #2397
   
----
commit 89e8c4ce3a306982d38bb088a61af9408c617d4a
Author: Nelson Costa <[hidden email]>
Date:   2017-06-06T15:35:06Z

    [ZEPPELIN-2592] Ensure open stream is closed

commit 2c8d83c8f641a950519c23f1497fe6b4f83c8e2f
Author: Nelson Costa <[hidden email]>
Date:   2017-06-09T15:01:15Z

    [ZEPPELIN-2592] Ensure open stream is closed + refactoring

----


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2397: [ZEPPELIN-2592] Ensure open stream is closed

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user necosta commented on the issue:

    https://github.com/apache/zeppelin/pull/2397
 
    Ready for another review round. Thanks


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin pull request #2397: [ZEPPELIN-2592] Ensure open stream is closed

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user necosta closed the pull request at:

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


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin pull request #2397: [ZEPPELIN-2592] Ensure open stream is closed

prabhjyotsingh
In reply to this post by prabhjyotsingh
GitHub user necosta reopened a pull request:

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

    [ZEPPELIN-2592] Ensure open stream is closed

    ### What is this PR for?
    Fix for missing close statement on input streams found on Helium Bundle factory
   
    ### What type of PR is it?
    Bug Fix
   
    ### What is the Jira issue?
    * https://issues.apache.org/jira/browse/ZEPPELIN-2592
   
    ### How should this be tested?
    NA
   
    ### Screenshots (if appropriate)
   
    ### Questions:
    * Does the licenses files need update? N
    * Is there breaking changes for older versions? N
    * Does this needs documentation? N


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/nokia/zeppelin zeppelin2592

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

    https://github.com/apache/zeppelin/pull/2397.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 #2397
   
----
commit 89e8c4ce3a306982d38bb088a61af9408c617d4a
Author: Nelson Costa <[hidden email]>
Date:   2017-06-06T15:35:06Z

    [ZEPPELIN-2592] Ensure open stream is closed

commit 2c8d83c8f641a950519c23f1497fe6b4f83c8e2f
Author: Nelson Costa <[hidden email]>
Date:   2017-06-09T15:01:15Z

    [ZEPPELIN-2592] Ensure open stream is closed + refactoring

commit 2c266502cef6f7d9826c4d9d6e1eac5880d9ae76
Author: Nelson Costa <[hidden email]>
Date:   2017-06-10T20:40:36Z

    ZEPPELIN-2592] Fixed bug on target directory

commit 003321e577cbd36a86240168b3e8042f51d24f7d
Author: Nelson Costa <[hidden email]>
Date:   2017-06-11T09:10:14Z

    Revert "ZEPPELIN-2592] Fixed bug on target directory"
   
    This reverts commit 2c266502cef6f7d9826c4d9d6e1eac5880d9ae76.

----


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2397: [ZEPPELIN-2592] Ensure open stream is closed

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user necosta commented on the issue:

    https://github.com/apache/zeppelin/pull/2397
 
    @felixcheung LG, but this is a fair bit more than ensure open stream is closed, so leave for other people to comment...  
    I agree. This whole class is very new and work-in-progress. I'm using it and testing it in detail so trying to get out a more stable version. Just as a note, I pushed and reverted a commit on a bit of code that confused me. Looks that the npm package always needs to have root folder name "package". I'm keeping that logic.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2397: [ZEPPELIN-2592] Ensure open stream is closed

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user necosta commented on the issue:

    https://github.com/apache/zeppelin/pull/2397
 
    @felixcheung , @jongyoul , @Leemoonsoo , can this be merged? Thanks.


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin issue #2397: [ZEPPELIN-2592] Ensure open stream is closed

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user Leemoonsoo commented on the issue:

    https://github.com/apache/zeppelin/pull/2397
 
    LGTM and merge to master if no further 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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|

[GitHub] zeppelin pull request #2397: [ZEPPELIN-2592] Ensure open stream is closed

prabhjyotsingh
In reply to this post by prabhjyotsingh
Github user asfgit closed the pull request at:

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


---
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 [hidden email] or file a JIRA ticket
with INFRA.
---