Added file option to generate_cluster_xml #121

Merged
merged 2 commits into from Feb 12, 2013

3 participants

@baepiff

When I used 0.90.1 I could create cluster.xml as below,

python generate_cluster_xml.py myHostList.txt 20 > cluster.xml

But from 0.96, I had to edit all host name in cluster.xml. So I added file option. The file is supposed to have host name list or server's IP addresses.

Here is the usage.

python generate_cluster_xml -f myHostList.txt -p 20 > cluster.xml

@vinothchandar
Collaborator

I am assigning this to Jay, who has the most recent context around this..

@jayjwylie jayjwylie was assigned Feb 6, 2013
@jayjwylie jayjwylie commented on an outdated diff Feb 8, 2013
bin/generate_cluster_xml.py
@@ -44,7 +46,11 @@
sys.exit(1)
# Store arguments
-nodes = args.nodes
+if args.file:
+ hostList = open(agrs.file).readlines()
@jayjwylie
jayjwylie added a line comment Feb 8, 2013

Please fix variable name 'agrs':
hostList = open(args.file).readlines()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jayjwylie jayjwylie commented on an outdated diff Feb 8, 2013
bin/generate_cluster_xml.py
@@ -10,6 +10,8 @@
# Setup and argument parser
parser = argparse.ArgumentParser(description='Build a voldemort cluster.xml.')
# Add supported arguments
+parser.add_argument('-f', '--file', type=str, dest='file',
+ help='the file of the list of hosts')
@jayjwylie
jayjwylie added a line comment Feb 8, 2013

Please be more specific in the help:

help='the file of the list of hosts (one per line)')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jayjwylie jayjwylie commented on the diff Feb 8, 2013
bin/generate_cluster_xml.py
@@ -73,7 +79,10 @@
print " <server>"
print " <id>%d</id>" % i
- print " <host>host%d</host>" % i
+ if args.file:
+ print " <host>%s</host>" % hostList[i]
@jayjwylie
jayjwylie added a line comment Feb 8, 2013

Please strip out the new line from the hosts file:
print " %s" % hostList[i].strip()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jayjwylie

I added some minor comments. When these minor issues are fixed, I can merge the pull request.

Thanks!
-Jay

@baepiff

Thank you for your review. Let me fix it!

@baepiff

Thank you for your comments. I fixed them. Please check them again.

Thanks,
Joongjin

@jayjwylie jayjwylie merged commit c94b8c5 into voldemort:master Feb 12, 2013
@jayjwylie jayjwylie was unassigned by baepiff Dec 1, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment