You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by prabhjyotsingh <gi...@git.apache.org> on 2015/12/29 07:07:50 UTC

[GitHub] incubator-zeppelin pull request: ZEPPELIN-541 - [WIP] increase log...

GitHub user prabhjyotsingh opened a pull request:

    https://github.com/apache/incubator-zeppelin/pull/579

    ZEPPELIN-541 - [WIP] increase logging in application

    ### What is this PR for?
    To increase logging in the application, so as it 
    
    ### What type of PR is it?
    Refactoring
    
    ### Todos
    * [x] - replace all printStackTrace() to LOGGER.error
    * [ ] - all catch block that doesn't throw exception, should have logging
    
    
    ### Is there a relevant Jira issue?
    Yes, ZEPPELIN-541
    
    
    ### Screenshots (if appropriate)
    N/A

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

    $ git pull https://github.com/prabhjyotsingh/incubator-zeppelin increaseLogging

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

    https://github.com/apache/incubator-zeppelin/pull/579.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 #579
    
----
commit d1b4ae29307f5ba53d1df1814dbfd8324275b2ec
Author: Prabhjyot Singh <pr...@gmail.com>
Date:   2015-12-29T05:55:57Z

    replace printStackTrace() to LOGGER.error

----


---
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-541 - [WIP] increase log...

Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/579#issuecomment-167943637
  
    @prabhjyotsingh Thanks for the quick fix. 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-541 - increase logging i...

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

    https://github.com/apache/incubator-zeppelin/pull/579


---
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-541 - [WIP] increase log...

Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/579#issuecomment-167960756
  
    +1 @HeartSaVioR 


---
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-541 - [WIP] increase log...

Posted by prabhjyotsingh <gi...@git.apache.org>.
Github user prabhjyotsingh commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/579#issuecomment-167944109
  
    @HeartSaVioR agreed, I was already half way doing this.


---
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-541 - [WIP] increase log...

Posted by prabhjyotsingh <gi...@git.apache.org>.
Github user prabhjyotsingh commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/579#issuecomment-167940151
  
    @jongyoul, @HeartSaVioR thanks for the quick feed back, have reverted the variable name back to what they were.


---
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-541 - increase logging i...

Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/579#issuecomment-170316842
  
    LGTM. Merging if there isn't no more discussion.


---
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-541 - [WIP] increase log...

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/579#issuecomment-167944033
  
    @prabhjyotsingh @jongyoul 
    It would be better to also leave stack trace to logger message. 
    `LOGGER.error(e.toString());` -> `LOGGER.error(e.toString(), e);`
    What do you think?


---
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-541 - increase logging i...

Posted by prabhjyotsingh <gi...@git.apache.org>.
Github user prabhjyotsingh commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/579#issuecomment-169226591
  
    Fixed CI, ready for 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-541 - [WIP] increase log...

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/579#issuecomment-167901389
  
    +1 to @jongyoul 
    Before changing logger field name to be consistent, policy should be built first.


---
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-541 - [WIP] increase log...

Posted by jongyoul <gi...@git.apache.org>.
Github user jongyoul commented on the pull request:

    https://github.com/apache/incubator-zeppelin/pull/579#issuecomment-167823570
  
    @prabhjyotsingh Thanks for the fixing this. It's good to me. But I think that changing the logger name from ```logger``` and ```LOG``` to ```LOGGER``` is irrelevant. Although Zeppelin doesn't get any policy to take a name for logger, it's better for now to reserve original name. Could you please fix this? Except that, 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.
---