generate_cluster_xml.py enhancements #86

Merged
merged 5 commits into from Jul 27, 2012

Projects

None yet

3 participants

@stotch
Collaborator
stotch commented Jul 20, 2012

Examples of supported use, now ...
https://gist.github.com/ed407ccd4b83ec3a5ed6

It's not pretty but it works better. The differences with these changes are ...

  • Has help
  • Cleaner, more flexible output that includes a comment with the seed it used to generate the distribution
  • Defaults to a 2-node 600-partition cluster
  • All defaults can be overridden (including forcing a user-defined seed) using arguments from long or short options
  • Limited support for zones (will plug zone-ids into hosts, but you have to add the fields to the cluster yourself)
  • Partitions are now thrown into a dictionary for easier reuse and manipulation later
  • Script is easier to add on to and maintain
stotch added some commits Jul 11, 2012
@stotch stotch Added default behavior, handling of long and short options for modify…
…ing defaults, zone support, random seed generation, removed text width formatting as it is not necessary, cleaned up format, refactored code, sys module no longer necessary (now that argparse is used) so removed it, added default interpreter path
962298e
@stotch stotch Fixing typo a560629
@stotch stotch Fixed --seed argument handling so that it actually works, now 3aa9d12
@stotch stotch Getting rid of some unused and deprecated code, added in a comment to…
… help explain and separate the partition shuffling
415d982
@jayjwylie jayjwylie was assigned Jul 24, 2012
@jayjwylie
Contributor

This looks mostly good. There are a couple of typos to be fixed in comments (clusteer and assining). A dictionary seems to be used unnecessarily (part_map = dict() seems unnecessary). Diff of changes I would make is below:

diff --git a/bin/generate_cluster_xml.py b/bin/generate_cluster_xml.py
index dd1187d..66ed514 100644
--- a/bin/generate_cluster_xml.py
+++ b/bin/generate_cluster_xml.py
@@ -11,7 +11,8 @@ rseed = int(random.randint(00000000001,99999999999))
 parser = argparse.ArgumentParser(description='Build a voldemort cluster.xml.')
 # Add supported arguments
 parser.add_argument('-N', '--name', type=str, default='voldemort', dest='name',
-                    help='the name you want to give the clusteer')
+# DIFF                    help='the name you want to give the clusteer')
+                    help='the name you want to give the cluster')
 parser.add_argument('-n', '--nodes', type=int, default=2, dest='nodes',
                     help='the number of nodes in the cluster')
 parser.add_argument('-p', '--partitions', type=int, default=300,
@@ -63,14 +64,16 @@ if args.zones:
 random.seed(seed)
 random.shuffle(part_ids)
 
-# Assining partitions to nodes and printing cluster.xml
-part_map = dict()
+# DIFF # Assining partitions to nodes and printing cluster.xml
+# Assigning partitions to nodes and printing cluster.xml
+# DIFF part_map = dict()
 print "" % seed
 print ""
 print "  %s" % name
 
 for i in xrange(nodes):
-  part_map[i] = ", ".join(str(p) for p in sorted(part_ids[i*partitions:(i+1)*partitions]))
+# DIFF  part_map[i] = ", ".join(str(p) for p in sorted(part_ids[i*partitions:(i+1)*partitions]))
+  part_map = ", ".join(str(p) for p in sorted(part_ids[i*partitions:(i+1)*partitions]))
 
   print "  "
   print "    %d" % i
@@ -78,7 +81,8 @@ for i in xrange(nodes):
   print "    %d" % http_port
   print "    %d" % sock_port
   print "    %d" % admin_port
-  print "    %s" % part_map[i]
+# DIFF  "    %s" % part_map[i]
+  print "    %s" % part_map
   # If zones are being used, assign a zone-id
   if args.zones:
     print "    %d" % zone_id
@stotch
Collaborator
stotch commented Jul 26, 2012

How about this, instead?


diff --git a/bin/generate_cluster_xml.py b/bin/generate_cluster_xml.py
index dd1187d..1811fdd 100644
--- a/bin/generate_cluster_xml.py
+++ b/bin/generate_cluster_xml.py
@@ -11,7 +11,7 @@ rseed = int(random.randint(00000000001,99999999999))
 parser = argparse.ArgumentParser(description='Build a voldemort cluster.xml.')
 # Add supported arguments
 parser.add_argument('-N', '--name', type=str, default='voldemort', dest='name',
-                    help='the name you want to give the clusteer')
+                    help='the name you want to give the cluster')
 parser.add_argument('-n', '--nodes', type=int, default=2, dest='nodes',
                     help='the number of nodes in the cluster')
 parser.add_argument('-p', '--partitions', type=int, default=300,
@@ -63,14 +63,13 @@ if args.zones:
 random.seed(seed)
 random.shuffle(part_ids)
 
-# Assining partitions to nodes and printing cluster.xml
-part_map = dict()
+# Printing cluster.xml
 print "" % seed
 print ""
 print "  %s" % name
 
 for i in xrange(nodes):
-  part_map[i] = ", ".join(str(p) for p in sorted(part_ids[i*partitions:(i+1)*pa
+  node_partitions = ", ".join(str(p) for p in sorted(part_ids[i*partitions:(i+1
 
   print "  "
   print "    %d" % i
@@ -78,7 +77,7 @@ for i in xrange(nodes):
   print "    %d" % http_port
   print "    %d" % sock_port
   print "    %d" % admin_port
-  print "    %s" % part_map[i]
+  print "    %s" % node_partitions
   # If zones are being used, assign a zone-id
   if args.zones:
     print "    %d" % zone_id

Same as you suggested, but slightly reworded/renamed to keep things relevant.

@jayjwylie
Contributor

Yeah. That looks good. I'll figure out the right button on git to pull
this into master once you've pushed. (Not sure if I used the right
verbs, but you know what I mean...)
-Jay

On Thu, Jul 26, 2012 at 4:31 PM, Brendan Harris
reply@reply.github.com
wrote:

How about this, instead?


diff --git a/bin/generate_cluster_xml.py b/bin/generate_cluster_xml.py
index dd1187d..1811fdd 100644
--- a/bin/generate_cluster_xml.py
+++ b/bin/generate_cluster_xml.py
@@ -11,7 +11,7 @@ rseed = int(random.randint(00000000001,99999999999))
 parser = argparse.ArgumentParser(description='Build a voldemort cluster.xml.')
 # Add supported arguments
 parser.add_argument('-N', '--name', type=str, default='voldemort', dest='name',
-                    help='the name you want to give the clusteer')
+                    help='the name you want to give the cluster')
 parser.add_argument('-n', '--nodes', type=int, default=2, dest='nodes',
                     help='the number of nodes in the cluster')
 parser.add_argument('-p', '--partitions', type=int, default=300,
@@ -63,14 +63,13 @@ if args.zones:
 random.seed(seed)
 random.shuffle(part_ids)

-# Assining partitions to nodes and printing cluster.xml
-part_map = dict()
+# Printing cluster.xml
 print "" % seed
 print ""
 print "  %s" % name

 for i in xrange(nodes):
-  part_map[i] = ", ".join(str(p) for p in sorted(part_ids[i*partitions:(i+1)*pa
+  node_partitions = ", ".join(str(p) for p in sorted(part_ids[i*partitions:(i+1

   print "  "
   print "    %d" % i
@@ -78,7 +77,7 @@ for i in xrange(nodes):
   print "    %d" % http_port
   print "    %d" % sock_port
   print "    %d" % admin_port
-  print "    %s" % part_map[i]
+  print "    %s" % node_partitions
   # If zones are being used, assign a zone-id
   if args.zones:
     print "    %d" % zone_id

Same as you suggested, but slightly reworded/renamed to keep things relevant.


Reply to this email directly or view it on GitHub:
#86 (comment)

Jay Wylie
Jay.J.Wylie@gmail.com

@shingon
shingon commented Jul 27, 2012

Does this script require Python 2.7?

@stotch
Collaborator
stotch commented Jul 27, 2012

You can run it with python 2.6.x if you install argparse manually, otherwise you need 2.7+. argparse is the only 2.7 component in it.

@jayjwylie
Contributor

This looks good to me now. Hopefully, the note here about installing argparse module for Python 2.6 is sufficient.

@jayjwylie jayjwylie merged commit 0aa7f42 into voldemort:master Jul 27, 2012
@jayjwylie jayjwylie was unassigned by stotch Dec 1, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment