You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by Will McQueen <wi...@cloudera.com> on 2012/02/28 02:09:05 UTC

Review Request: Several issues with flume-ng script

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4076/
-----------------------------------------------------------

Review request for Flume.


Summary
-------

The changes in the script are described below:

1) Move FLUME_NODE_CLASS and FLUME_AVRO_CLIENT_CLASS after the sourcing of flume-env.sh, since there's no reason that flume-env.sh should override these 2 classes that are fundamental to Flume NG

2) The check for existence of flume-env.sh should include a check of whether $FLUME_CONF_DIR is non-null. Otherwise there's a (very rare) chance that a flume-env.sh file will be picked-up from root dir at '/flume-env.sh' if FLUME_CONF_DIR isn't set.

3) spelling: " !" => "!"

4) Change display_help() to follow standard syntax of [COMMAND] [OPION]... seen in man pages. Modify some indentation, based on new help entries

5) Remove all cases of using "--no-env,-E". This is not implemented.

6) Remove all cases of using "--data,-d". This is not implemented.

7) In display_help(), add node options and a node-specific help option. These node options are what you would see in the help output if you ran the Application.java class directly with "java Application -h"

8) In display_help(), add avro-client options. These client options are what you would see in the help output if you ran "AvroCLIClient -h".

9) In run_node() and run_avro_client(), make FLUME_APPLICATION_CLASS local to emphasize that its value isn't used in the caller's env. Also, remove the check for JAVA_HOME since it's guaranteed that JAVA_HOME will have been set by the time this function is called. In run_avro_client(), remove the FLUME_CLASSPATH line since a similar check was already done earlier in the same function.

10) Remove validate_env(), since --no-env,-E isn't implemented 


This addresses bug FLUME-1005.
    https://issues.apache.org/jira/browse/FLUME-1005


Diffs
-----

  bin/flume-ng 4670d92 

Diff: https://reviews.apache.org/r/4076/diff


Testing
-------

Running script


Thanks,

Will


Re: Review Request: Several issues with flume-ng script

Posted by Arvind Prabhakar <ar...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4076/#review5376
-----------------------------------------------------------

Ship it!


+1

- Arvind


On 2012-02-28 01:09:05, Will McQueen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4076/
> -----------------------------------------------------------
> 
> (Updated 2012-02-28 01:09:05)
> 
> 
> Review request for Flume.
> 
> 
> Summary
> -------
> 
> The changes in the script are described below:
> 
> 1) Move FLUME_NODE_CLASS and FLUME_AVRO_CLIENT_CLASS after the sourcing of flume-env.sh, since there's no reason that flume-env.sh should override these 2 classes that are fundamental to Flume NG
> 
> 2) The check for existence of flume-env.sh should include a check of whether $FLUME_CONF_DIR is non-null. Otherwise there's a (very rare) chance that a flume-env.sh file will be picked-up from root dir at '/flume-env.sh' if FLUME_CONF_DIR isn't set.
> 
> 3) spelling: " !" => "!"
> 
> 4) Change display_help() to follow standard syntax of [COMMAND] [OPION]... seen in man pages. Modify some indentation, based on new help entries
> 
> 5) Remove all cases of using "--no-env,-E". This is not implemented.
> 
> 6) Remove all cases of using "--data,-d". This is not implemented.
> 
> 7) In display_help(), add node options and a node-specific help option. These node options are what you would see in the help output if you ran the Application.java class directly with "java Application -h"
> 
> 8) In display_help(), add avro-client options. These client options are what you would see in the help output if you ran "AvroCLIClient -h".
> 
> 9) In run_node() and run_avro_client(), make FLUME_APPLICATION_CLASS local to emphasize that its value isn't used in the caller's env. Also, remove the check for JAVA_HOME since it's guaranteed that JAVA_HOME will have been set by the time this function is called. In run_avro_client(), remove the FLUME_CLASSPATH line since a similar check was already done earlier in the same function.
> 
> 10) Remove validate_env(), since --no-env,-E isn't implemented 
> 
> 
> This addresses bug FLUME-1005.
>     https://issues.apache.org/jira/browse/FLUME-1005
> 
> 
> Diffs
> -----
> 
>   bin/flume-ng 4670d92 
> 
> Diff: https://reviews.apache.org/r/4076/diff
> 
> 
> Testing
> -------
> 
> Running script
> 
> 
> Thanks,
> 
> Will
> 
>