----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36649/#review92460 -----------------------------------------------------------
contrib/agent-simulator/Docker/Yum_Dockerfile (line 21) <https://reviews.apache.org/r/36649/#comment146646> Can remove MAINTAINER element contrib/agent-simulator/Docker/package_list.txt (line 1) <https://reviews.apache.org/r/36649/#comment146647> Why is this needed? I see that yum commands will execute on every single line of it; is that for your custom image? contrib/agent-simulator/README.md (line 69) <https://reviews.apache.org/r/36649/#comment146649> Is this internal or external IP? contrib/agent-simulator/README.md (line 70) <https://reviews.apache.org/r/36649/#comment146650> What's the full path to config.ini? contrib/agent-simulator/README.md (line 72) <https://reviews.apache.org/r/36649/#comment146651> We should be using property names in all lowercase. contrib/agent-simulator/README.md (line 78) <https://reviews.apache.org/r/36649/#comment146652> How do we get the Weave IP of the server? contrib/agent-simulator/cluster.py (line 25) <https://reviews.apache.org/r/36649/#comment146655> Thanks for adding a logger. contrib/agent-simulator/cluster.py (line 34) <https://reviews.apache.org/r/36649/#comment146642> I should have caught this the first time around. Can we switch all of these variables and functions to lowercase? VM_list Weave_domain_name GCE_info_file_name __extract_VM_FQDN_IP__ request_GCE_cluster etc. contrib/agent-simulator/cluster.py (line 66) <https://reviews.apache.org/r/36649/#comment146653> "lines" should be initialized before the "with" statement since the for loop below it calls it. contrib/agent-simulator/cluster.py (line 73) <https://reviews.apache.org/r/36649/#comment146639> Why is the first element in the list excluded? contrib/agent-simulator/cluster.py (line 82) <https://reviews.apache.org/r/36649/#comment146654> Should initialize "lines" before "with" statement. contrib/agent-simulator/cluster.py (line 103) <https://reviews.apache.org/r/36649/#comment146640> Thanks for switching to format. All of these property names should be using lowercase. contrib/agent-simulator/config.py (line 41) <https://reviews.apache.org/r/36649/#comment146641> Can we switch this key to be all lower case? contrib/agent-simulator/config.py (line 46) <https://reviews.apache.org/r/36649/#comment146643> Can use os.path.join( Config.ATTRIBUTES["Output_folder"], Config.ATTRIBUTES[key]) contrib/agent-simulator/example/config.ini (line 21) <https://reviews.apache.org/r/36649/#comment146648> all of these properties and sections should be using lowercase to prevent any typos. contrib/agent-simulator/log.py (line 21) <https://reviews.apache.org/r/36649/#comment146656> Actually, can use Logging class with a circular logger. This is more robust, and can also have different logging levels. https://docs.python.org/2.3/lib/node304.html contrib/agent-simulator/log.py (line 27) <https://reviews.apache.org/r/36649/#comment146645> Can we switch this to all lowercase? contrib/agent-simulator/vm.py (line 36) <https://reviews.apache.org/r/36649/#comment146657> All of these "private" functions that begin with "__" don't need to end in "__". Actually, only one "_" is needed at the beginning. - Alejandro Fernandez On July 21, 2015, 7:07 p.m., Pengcheng Xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36649/ > ----------------------------------------------------------- > > (Updated July 21, 2015, 7:07 p.m.) > > > Review request for Ambari and Alejandro Fernandez. > > > Bugs: AMBARI-12339 > https://issues.apache.org/jira/browse/AMBARI-12339 > > > Repository: ambari > > > Description > ------- > > Code clean; Documentation; Add more features about Docker networking and > image building > > > Diffs > ----- > > contrib/agent-simulator/Docker/Dockerfile d22435a > contrib/agent-simulator/Docker/Raw_Dockerfile PRE-CREATION > contrib/agent-simulator/Docker/Yum_Dockerfile PRE-CREATION > contrib/agent-simulator/Docker/ambari_agent_start.sh 4ba2361 > contrib/agent-simulator/Docker/launcher_agent.py 5ff008d > contrib/agent-simulator/Docker/package_list.txt PRE-CREATION > contrib/agent-simulator/Linux/CentOS7/docker_install.sh 033c2b9 > contrib/agent-simulator/Linux/CentOS7/set_docker_partition.sh PRE-CREATION > contrib/agent-simulator/Linux/CentOS7/weave_install.sh 6788184 > contrib/agent-simulator/Linux/Ubuntu12/docker_install.sh 220f1a4 > contrib/agent-simulator/Linux/Ubuntu12/weave_install.sh 332488d > contrib/agent-simulator/README.md PRE-CREATION > contrib/agent-simulator/cluster.py 55c3c44 > contrib/agent-simulator/config.py 20cc8de > contrib/agent-simulator/config/config.ini a3afa33 > contrib/agent-simulator/docker.py e397c4b > contrib/agent-simulator/example/config.ini PRE-CREATION > contrib/agent-simulator/launcher_cluster.py 0a7cbe0 > contrib/agent-simulator/launcher_docker.py 48e8fac > contrib/agent-simulator/log.py PRE-CREATION > contrib/agent-simulator/network/DNS_editor.py PRE-CREATION > contrib/agent-simulator/network/set_ambari_server_network.sh PRE-CREATION > contrib/agent-simulator/network/set_host_network.sh PRE-CREATION > contrib/agent-simulator/server/ambari_server_install.sh PRE-CREATION > contrib/agent-simulator/server/ambari_server_reset_data.sh PRE-CREATION > contrib/agent-simulator/server_setup.sh 3b499aa > contrib/agent-simulator/tips.txt 207177d > contrib/agent-simulator/vm.py 90de4de > > Diff: https://reviews.apache.org/r/36649/diff/ > > > Testing > ------- > > Manually tested on Google Compute > > > Thanks, > > Pengcheng Xu > >
