Uploaded image for project: 'Chukwa'
  1. Chukwa
  2. CHUKWA-270

undesired ChukwaAgentController message injected to stdout of the running program

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.3.0
    • Fix Version/s: 0.3.0
    • Component/s: Data Collection
    • Labels:
      None
    • Environment:

      Redhat EL 5.1, Java 6

      Description

      Agent Controller currently prints it's messages to the stdout of the running program. This should be changed to using logger to prevent messages showing up in the unexpected places.

      1. ASF.LICENSE.NOT.GRANTED--ChukwaAgentController.java
        21 kB
        rushin barot
      2. CHUKWA-270.patch
        7 kB
        rushin barot
      3. CHUKWA-270-updated.patch
        6 kB
        rushin barot

        Activity

        Hide
        rushinbarotsjsu rushin barot added a comment -

        Hi Eric,

        I tried to commit file but i get following errors

        commit -m "CHUKWA-270 : Agent Controller's stdout has been changed to
        logger to prevent messages showing up in the unexpected places."
        /media/disk/Documents and Settings/Rushin Barot/My
        Documents/2009/hadoop/Chukwajava/src/java/org/apache/hadoop/chukwa/datacollection/controller/ChukwaAgentController.java
        svn: Commit failed (details follow):
        svn: Commit failed (details follow):
        svn: MKACTIVITY of
        '/repos/asf/!svn/act/a27ff1a0-2201-0010-80cd-9b294cb31e66': 403 Forbidden (
        http://svn.apache.org)

        It looks like I don't have rights to check in. please find my attached file.

        Thank your for your guidance.


        Best regards,
        Rushin Barot

        Show
        rushinbarotsjsu rushin barot added a comment - Hi Eric, I tried to commit file but i get following errors commit -m " CHUKWA-270 : Agent Controller's stdout has been changed to logger to prevent messages showing up in the unexpected places." /media/disk/Documents and Settings/Rushin Barot/My Documents/2009/hadoop/Chukwajava/src/java/org/apache/hadoop/chukwa/datacollection/controller/ChukwaAgentController.java svn: Commit failed (details follow): svn: Commit failed (details follow): svn: MKACTIVITY of '/repos/asf/!svn/act/a27ff1a0-2201-0010-80cd-9b294cb31e66': 403 Forbidden ( http://svn.apache.org ) It looks like I don't have rights to check in. please find my attached file. Thank your for your guidance. – Best regards, Rushin Barot
        Hide
        asrabkin Ari Rabkin added a comment -

        Hey, Rushin.

        In general, Apache projects tend to have many contributors and few committers. The usual process is that if you have a patch, you post it as an attachment, mark it as available, and get someone (not necessarily a committer) to review it. Once it's been approved, one of the committers will get to it and check it in, usually within a day or two.

        To generate a patch, you say "svn diff [files you want in the patch] > patchfilenanme.patch". Then post just the patch, not the whole modified file. This makes it clearer what's being changed and prevents extraneous and unexpected changes from creeping in.

        One other process note. If at all possible, your patch should include new or modified unit tests to demonstrate that it's functioning correctly. That's probably neither feasible nor useful here, so don't worry about it on this issue.

        This may sound like a fair bit of trouble for a small and simple change. But that's the way Hadoop, Chukwa, and I believe all apache projects are run. It makes it much, MUCH easier to keep the codebase stable and working correctly.

        Show
        asrabkin Ari Rabkin added a comment - Hey, Rushin. In general, Apache projects tend to have many contributors and few committers. The usual process is that if you have a patch, you post it as an attachment, mark it as available, and get someone (not necessarily a committer) to review it. Once it's been approved, one of the committers will get to it and check it in, usually within a day or two. To generate a patch, you say "svn diff [files you want in the patch] > patchfilenanme.patch". Then post just the patch, not the whole modified file. This makes it clearer what's being changed and prevents extraneous and unexpected changes from creeping in. One other process note. If at all possible, your patch should include new or modified unit tests to demonstrate that it's functioning correctly. That's probably neither feasible nor useful here, so don't worry about it on this issue. This may sound like a fair bit of trouble for a small and simple change. But that's the way Hadoop, Chukwa, and I believe all apache projects are run. It makes it much, MUCH easier to keep the codebase stable and working correctly.
        Hide
        asrabkin Ari Rabkin added a comment -

        Patch looks good. My one hesitation is that I noticed you use log.error for the usage statement. This may not be ideal – I think that really should go to standard out. Thoughts?

        For your reference: when you have a patch ready for review, it's helpful to mark the patch as available, using the "submit patch" link on the left. This makes it easy for reviewers to find reviewable patches, and for committers to find committable ones..

        Show
        asrabkin Ari Rabkin added a comment - Patch looks good. My one hesitation is that I noticed you use log.error for the usage statement. This may not be ideal – I think that really should go to standard out. Thoughts? For your reference: when you have a patch ready for review, it's helpful to mark the patch as available, using the "submit patch" link on the left. This makes it easy for reviewers to find reviewable patches, and for committers to find committable ones..
        Hide
        rushinbarotsjsu rushin barot added a comment -

        I thought rather than crashing program it should go to log.error. should i change it to standard out?

        Show
        rushinbarotsjsu rushin barot added a comment - I thought rather than crashing program it should go to log.error. should i change it to standard out?
        Hide
        rushinbarotsjsu rushin barot added a comment -

        Rewriting my comments

        I thought it should better go to file rather than console.

        Show
        rushinbarotsjsu rushin barot added a comment - Rewriting my comments I thought it should better go to file rather than console.
        Hide
        jboulon Jerome Boulon added a comment -

        +1 on writing to console stdout since it's from the main.
        Also could you regenerate your patch from CHUKWA_HOME directory?

        Show
        jboulon Jerome Boulon added a comment - +1 on writing to console stdout since it's from the main. Also could you regenerate your patch from CHUKWA_HOME directory?
        Hide
        rushinbarotsjsu rushin barot added a comment -
        • updated log.error to stdout
        • updated patch from Chukwa_ home dir
        Show
        rushinbarotsjsu rushin barot added a comment - updated log.error to stdout updated patch from Chukwa_ home dir
        Hide
        asrabkin Ari Rabkin added a comment -

        I just committed this. Thanks, Rushin, and welcome to Chukwa!

        Show
        asrabkin Ari Rabkin added a comment - I just committed this. Thanks, Rushin, and welcome to Chukwa!

          People

          • Assignee:
            rushinbarotsjsu rushin barot
            Reporter:
            eyang Eric Yang
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development