You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apex.apache.org by davidyan74 <gi...@git.apache.org> on 2016/05/20 22:06:06 UTC

[GitHub] incubator-apex-core pull request: APEXCORE-466 #resolve improved l...

GitHub user davidyan74 opened a pull request:

    https://github.com/apache/incubator-apex-core/pull/339

    APEXCORE-466 #resolve improved logging from the *Agent.java files

    @PramodSSImmaneni please merge

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

    $ git pull https://github.com/davidyan74/incubator-apex-core APEXCORE-466

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

    https://github.com/apache/incubator-apex-core/pull/339.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 #339
    
----
commit c4497e454c7e96e36dd0db274c16d5717fb03322
Author: David Yan <da...@datatorrent.com>
Date:   2016-05-20T22:04:56Z

    APEXCORE-466 #resolve improved logging from the *Agent.java files

----


---
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-apex-core pull request #339: APEXCORE-466 #resolve improved loggin...

Posted by gauravgopi123 <gi...@git.apache.org>.
Github user gauravgopi123 commented on a diff in the pull request:

    https://github.com/apache/incubator-apex-core/pull/339#discussion_r65485278
  
    --- Diff: engine/src/main/java/com/datatorrent/stram/client/EventsAgent.java ---
    @@ -125,7 +125,7 @@ protected EventsIndexLine parseIndexLine(String line) throws JSONException
             totalNumEvents += indexLine.numEvents;
           }
         } catch (Exception ex) {
    -      LOG.warn("Got exception when reading events", ex);
    +      LOG.warn("Cannot read events for {}: {}", appId, ex.toString());
    --- End diff --
    
    @davidyan74 : Do you need to do ex.toString()?


---
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-apex-core pull request #339: APEXCORE-466 #resolve improved loggin...

Posted by gauravgopi123 <gi...@git.apache.org>.
Github user gauravgopi123 commented on a diff in the pull request:

    https://github.com/apache/incubator-apex-core/pull/339#discussion_r65788854
  
    --- Diff: engine/src/main/java/com/datatorrent/stram/client/RecordingsAgent.java ---
    @@ -640,7 +640,7 @@ public JSONObject process(WebResource.Builder webResource, Class<JSONObject> cla
               }
           );
         } catch (Exception ex) {
    -      LOG.error("Exception caught", ex);
    +      LOG.error("Cannot start recording: ", ex);
    --- End diff --
    
    @davidyan74 : Should logging be not done at same levels? I see here it is "error" and earlier it is done at "warn" and also this will print stacktrace but log messages at 546 line won't...
    
    Any reason for inconsistent log levels?


---
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-apex-core pull request #339: APEXCORE-466 #resolve improved loggin...

Posted by davidyan74 <gi...@git.apache.org>.
Github user davidyan74 commented on a diff in the pull request:

    https://github.com/apache/incubator-apex-core/pull/339#discussion_r65610705
  
    --- Diff: engine/src/main/java/com/datatorrent/stram/client/EventsAgent.java ---
    @@ -125,7 +125,7 @@ protected EventsIndexLine parseIndexLine(String line) throws JSONException
             totalNumEvents += indexLine.numEvents;
           }
         } catch (Exception ex) {
    -      LOG.warn("Got exception when reading events", ex);
    +      LOG.warn("Cannot read events for {}: {}", appId, ex);
    --- End diff --
    
    This is intentional. We don't want to print the stack.


---
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-apex-core issue #339: APEXCORE-466 #resolve improved logging from ...

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

    https://github.com/apache/incubator-apex-core/pull/339
  
    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-apex-core pull request #339: APEXCORE-466 #resolve improved loggin...

Posted by gauravgopi123 <gi...@git.apache.org>.
Github user gauravgopi123 commented on a diff in the pull request:

    https://github.com/apache/incubator-apex-core/pull/339#discussion_r65602951
  
    --- Diff: engine/src/main/java/com/datatorrent/stram/client/EventsAgent.java ---
    @@ -125,7 +125,7 @@ protected EventsIndexLine parseIndexLine(String line) throws JSONException
             totalNumEvents += indexLine.numEvents;
           }
         } catch (Exception ex) {
    -      LOG.warn("Got exception when reading events", ex);
    +      LOG.warn("Cannot read events for {}: {}", appId, ex);
    --- End diff --
    
    2nd '{ }' not required...


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