From cf74019667c24740473ba7b2180df7092335b5cd Mon Sep 17 00:00:00 2001 From: Ghais Issa Date: Tue, 24 Sep 2013 17:59:22 -0400 Subject: [PATCH 01/16] Ability to assign weights to queues. When using multiple queues it is now possible to pass a weight for each queue in the form of -queues=hight=2,low=1 which would result in a job being selected at random from [high, high, low] If no weights are provided queues are processed in the strict order they are provided. e.g. -queues=high,low would result in the queue high being checked first and if no jobs are present then we would check the queue low. Fixes gh-4 --- README.md | 2 +- flags.go | 10 ++++++- goworker.go | 2 +- poller.go | 8 ++++-- process.go | 15 ++++++++++ queues_flag.go | 49 ++++++++++++++++++++++++++++++-- queues_flag_test.go | 68 +++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 146 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 4da4638..8fb6507 100644 --- a/README.md +++ b/README.md @@ -132,7 +132,7 @@ end There are several flags which control the operation of the goworker client. -* `-queues="comma,delimited,queues"` — This is the only required flag. The recommended practice is to separate your Resque workers from your goworkers with different queues. Otherwise, Resque worker classes that have no goworker analog will cause the goworker process to fail the jobs. Because of this, there is no default queue, nor is there a way to select all queues (à la Resque's `*` queue). +* `-queues="comma,delimited,queues"` — This is the only required flag. The recommended practice is to separate your Resque workers from your goworkers with different queues. Otherwise, Resque worker classes that have no goworker analog will cause the goworker process to fail the jobs. Because of this, there is no default queue, nor is there a way to select all queues (à la Resque's `*` queue). If you have multiple queues you can assign them weights. A queue with a weight of 2 will be checked twice as often as a queue with a weight of 1: -queues='high=2,low=1' * `-interval=5.0` — Specifies the wait period between polling if no job was in the queue the last time one was requested. * `-concurrency=25` — Specifies the number of concurrently executing workers. This number can be as low as 1 or rather comfortably as high as 100,000, and should be tuned to your workflow and the availability of outside resources. * `-connections=2` — Specifies the maximum number of Redis connections that goworker will consume between the poller and all workers. There is not much performance gain over two and a slight penalty when using only one. This is configurable in case you need to keep connection counts low for cloud Redis providers who limit plans on `maxclients`. diff --git a/flags.go b/flags.go index ef13c25..fc9a1b2 100644 --- a/flags.go +++ b/flags.go @@ -19,7 +19,12 @@ // cause the goworker process to fail the jobs. // Because of this, there is no default queue, // nor is there a way to select all queues (à la -// Resque's * queue). +// Resque's * queue). Queues are processed in +// the order they are specififed. +// If you have multiple queues you can assign +// them weights. A queue with a weight of 2 will +// be checked twice as often as a queue with a +// weight of 1: -queues='high=2,low=1' // // -interval=5.0 // — Specifies the wait period between polling if @@ -75,6 +80,7 @@ package goworker import ( "flag" "os" + "strings" ) var ( @@ -87,6 +93,7 @@ var ( uri string namespace string exitOnComplete bool + isStrict bool ) func init() { @@ -125,5 +132,6 @@ func flags() error { if err := interval.SetFloat(intervalFloat); err != nil { return err } + isStrict = strings.IndexRune(queuesString, '=') == -1 return nil } diff --git a/goworker.go b/goworker.go index 9d44376..5406856 100644 --- a/goworker.go +++ b/goworker.go @@ -31,7 +31,7 @@ func Work() error { pool := newRedisPool(uri, connections, connections, time.Minute) defer pool.Close() - poller, err := newPoller(queues) + poller, err := newPoller(queues, isStrict) if err != nil { return err } diff --git a/poller.go b/poller.go index 061210d..a18eb8f 100644 --- a/poller.go +++ b/poller.go @@ -9,20 +9,22 @@ import ( type poller struct { process + isStrict bool } -func newPoller(queues []string) (*poller, error) { +func newPoller(queues []string, isStrict bool) (*poller, error) { process, err := newProcess("poller", queues) if err != nil { return nil, err } return &poller{ - process: *process, + process: *process, + isStrict: isStrict, }, nil } func (p *poller) getJob(conn *redisConn) (*job, error) { - for _, queue := range p.Queues { + for _, queue := range p.queues(p.isStrict) { logger.Debugf("Checking %s", queue) reply, err := conn.Do("LPOP", fmt.Sprintf("%squeue:%s", namespace, queue)) diff --git a/process.go b/process.go index b024c0e..b0c6fd3 100644 --- a/process.go +++ b/process.go @@ -2,6 +2,7 @@ package goworker import ( "fmt" + "math/rand" "os" "strings" "time" @@ -73,3 +74,17 @@ func (p *process) fail(conn *redisConn) error { return nil } + +func (p *process) queues(strict bool) []string { + //If the queues order is strict then just return them + if strict { + return p.Queues + } + + //If not then we want to to shuffle the queues before returning them + queues := make([]string, len(p.Queues)) + for i, v := range rand.Perm(len(p.Queues)) { + queues[i] = p.Queues[v] + } + return queues +} diff --git a/queues_flag.go b/queues_flag.go index bf2e878..6443bbb 100644 --- a/queues_flag.go +++ b/queues_flag.go @@ -3,24 +3,69 @@ package goworker import ( "errors" "fmt" + "strconv" "strings" ) var ( - errorEmptyQueues = errors.New("You must specify at least one queue.") + errorEmptyQueues = errors.New("You must specify at least one queue.") + errorNoneNumericWeight = errors.New("The weight must be a numeric value.") ) type queuesFlag []string func (q *queuesFlag) Set(value string) error { + if value == "" { return errorEmptyQueues } + //Parse the individual queues and their weights if they are present. + for _, queueAndWeight := range strings.Split(value, ",") { + queue, weight, err := parseQueueAndWeight(queueAndWeight) + if err != nil { + return err + } - *q = append(*q, strings.Split(value, ",")...) + for i := 0; i < weight; i++ { + *q = append(*q, queue) + } + } return nil } func (q *queuesFlag) String() string { return fmt.Sprint(*q) } + +func parseQueueAndWeight(queueAndWeight string) (queue string, weight int, err error) { + parts := strings.Split(queueAndWeight, "=") + // There must be exactly one '=' in queue/weight declaration + if len(parts) > 2 { + return "", 0, errorNoneNumericWeight + } + + //The empty string is a valid queue name, and has the default weight of 1 + if len(parts) == 0 { + return "", 1, nil + } + + //If '=' is not present then we only have the queue name and the default weight is 1 + if len(parts) == 1 { + queue = parts[0] + weight = 1 + err = nil + return + } + + //Check to see if we have a weight for this queue + if len(parts) == 2 { + queue = parts[0] + weight, err = strconv.Atoi(parts[1]) + if err != nil { + return "", 0, errorNoneNumericWeight + } + return queue, weight, nil + } + return + +} diff --git a/queues_flag_test.go b/queues_flag_test.go index d597dcc..5f2de16 100644 --- a/queues_flag_test.go +++ b/queues_flag_test.go @@ -26,6 +26,21 @@ var queuesFlagSetTests = []struct { queuesFlag([]string{"high", "low"}), nil, }, + { + "high=2,low=1", + queuesFlag([]string{"high", "high", "low"}), + nil, + }, + { + "high=2,low", + queuesFlag([]string{"high", "high", "low"}), + nil, + }, + { + "low=1,high=2", + queuesFlag([]string{"low", "high", "high"}), + nil, + }, } func TestQueuesFlagSet(t *testing.T) { @@ -63,3 +78,56 @@ func TestQueuesFlagString(t *testing.T) { } } } + +func TestParseQueueAndWeight(t *testing.T) { + for _, tt := range parseQueueAndWeightTests { + queue, weight, err := parseQueueAndWeight(tt.queueAndWeight) + if queue != tt.queue { + t.Errorf("parseQueueAndWeight#queue expected %s, actual %s", tt.queue, queue) + } + if weight != tt.weight { + t.Errorf("parseQueueAndWeight#weight expected %d, actual %d", tt.weight, weight) + } + if err != tt.err { + t.Errorf("parseQueueAndWeight#err expected %v, actual %v", tt.err, err) + } + } +} + +var parseQueueAndWeightTests = []struct { + queueAndWeight string + queue string + weight int + err error +}{ + { + "q==", + "", + 0, + errorNoneNumericWeight, + }, + { + "q=a", + "", + 0, + errorNoneNumericWeight, + }, + { + "", + "", + 1, + nil, + }, + { + "q", + "q", + 1, + nil, + }, + { + "q=2", + "q", + 2, + nil, + }, +} From fffcb54243cc68c56374beb4f786d6b3a3f3b894 Mon Sep 17 00:00:00 2001 From: Benjamin Manns Date: Wed, 25 Sep 2013 00:12:46 -0400 Subject: [PATCH 02/16] Fix error testing in TestQueuesFlagSet. This one was my fault. If the expected error is nil, we should check if the returned error is not nil. Fixing this lets us use the same test set for TestQueuesFlagSet and TestParseQueueAndWeight, since the final result is what we care about. --- queues_flag_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/queues_flag_test.go b/queues_flag_test.go index 5f2de16..8ead117 100644 --- a/queues_flag_test.go +++ b/queues_flag_test.go @@ -50,8 +50,10 @@ func TestQueuesFlagSet(t *testing.T) { if fmt.Sprint(actual) != fmt.Sprint(tt.expected) { t.Errorf("QueuesFlag: set to %s expected %v, actual %v", tt.v, tt.expected, actual) } - if tt.err != nil && err.Error() != tt.err.Error() { - t.Errorf("QueuesFlag: set to %s expected err %v, actual err %v", tt.v, tt.expected, actual) + if (err != nil && tt.err == nil) || + (err == nil && tt.err != nil) || + (err != nil && tt.err != nil && err.Error() != tt.err.Error()) { + t.Errorf("QueuesFlag: set to %s expected err %v, actual err %v", tt.v, tt.err, err) } } } From a9caad301bc1d18d1b113c24e3d66ff40fd06ac4 Mon Sep 17 00:00:00 2001 From: Benjamin Manns Date: Wed, 25 Sep 2013 00:14:50 -0400 Subject: [PATCH 03/16] ParseQueueAndWeight tests into TestQueuesFlagSet. Now that TestQueuesFlagSet is fixed, we can move the tests. --- queues_flag_test.go | 73 +++++++++++++-------------------------------- 1 file changed, 20 insertions(+), 53 deletions(-) diff --git a/queues_flag_test.go b/queues_flag_test.go index 8ead117..e672230 100644 --- a/queues_flag_test.go +++ b/queues_flag_test.go @@ -41,6 +41,26 @@ var queuesFlagSetTests = []struct { queuesFlag([]string{"low", "high", "high"}), nil, }, + { + "low=,high=2", + nil, + errors.New("The weight must be a numeric value."), + }, + { + "low=a,high=2", + nil, + errors.New("The weight must be a numeric value."), + }, + { + "low=", + nil, + errors.New("The weight must be a numeric value."), + }, + { + "low=a", + nil, + errors.New("The weight must be a numeric value."), + }, } func TestQueuesFlagSet(t *testing.T) { @@ -80,56 +100,3 @@ func TestQueuesFlagString(t *testing.T) { } } } - -func TestParseQueueAndWeight(t *testing.T) { - for _, tt := range parseQueueAndWeightTests { - queue, weight, err := parseQueueAndWeight(tt.queueAndWeight) - if queue != tt.queue { - t.Errorf("parseQueueAndWeight#queue expected %s, actual %s", tt.queue, queue) - } - if weight != tt.weight { - t.Errorf("parseQueueAndWeight#weight expected %d, actual %d", tt.weight, weight) - } - if err != tt.err { - t.Errorf("parseQueueAndWeight#err expected %v, actual %v", tt.err, err) - } - } -} - -var parseQueueAndWeightTests = []struct { - queueAndWeight string - queue string - weight int - err error -}{ - { - "q==", - "", - 0, - errorNoneNumericWeight, - }, - { - "q=a", - "", - 0, - errorNoneNumericWeight, - }, - { - "", - "", - 1, - nil, - }, - { - "q", - "q", - 1, - nil, - }, - { - "q=2", - "q", - 2, - nil, - }, -} From 5dc542aa29c9120c2d17d289987f87a8efa11d71 Mon Sep 17 00:00:00 2001 From: Benjamin Manns Date: Wed, 25 Sep 2013 00:15:21 -0400 Subject: [PATCH 04/16] Add markdown code highlighting to queues flag. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 8fb6507..26944b9 100644 --- a/README.md +++ b/README.md @@ -132,7 +132,7 @@ end There are several flags which control the operation of the goworker client. -* `-queues="comma,delimited,queues"` — This is the only required flag. The recommended practice is to separate your Resque workers from your goworkers with different queues. Otherwise, Resque worker classes that have no goworker analog will cause the goworker process to fail the jobs. Because of this, there is no default queue, nor is there a way to select all queues (à la Resque's `*` queue). If you have multiple queues you can assign them weights. A queue with a weight of 2 will be checked twice as often as a queue with a weight of 1: -queues='high=2,low=1' +* `-queues="comma,delimited,queues"` — This is the only required flag. The recommended practice is to separate your Resque workers from your goworkers with different queues. Otherwise, Resque worker classes that have no goworker analog will cause the goworker process to fail the jobs. Because of this, there is no default queue, nor is there a way to select all queues (à la Resque's `*` queue). If you have multiple queues you can assign them weights. A queue with a weight of 2 will be checked twice as often as a queue with a weight of 1: `-queues='high=2,low=1'` * `-interval=5.0` — Specifies the wait period between polling if no job was in the queue the last time one was requested. * `-concurrency=25` — Specifies the number of concurrently executing workers. This number can be as low as 1 or rather comfortably as high as 100,000, and should be tuned to your workflow and the availability of outside resources. * `-connections=2` — Specifies the maximum number of Redis connections that goworker will consume between the poller and all workers. There is not much performance gain over two and a slight penalty when using only one. This is configurable in case you need to keep connection counts low for cloud Redis providers who limit plans on `maxclients`. From 4c5fbeaf65ab8c3fba64289fbbaa173e5db68fa2 Mon Sep 17 00:00:00 2001 From: Benjamin Manns Date: Wed, 25 Sep 2013 00:17:06 -0400 Subject: [PATCH 05/16] Rename errorNoneNumericWeight to errorNonNumericWeight. --- queues_flag.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/queues_flag.go b/queues_flag.go index 6443bbb..adcefe8 100644 --- a/queues_flag.go +++ b/queues_flag.go @@ -8,8 +8,8 @@ import ( ) var ( - errorEmptyQueues = errors.New("You must specify at least one queue.") - errorNoneNumericWeight = errors.New("The weight must be a numeric value.") + errorEmptyQueues = errors.New("You must specify at least one queue.") + errorNonNumericWeight = errors.New("The weight must be a numeric value.") ) type queuesFlag []string @@ -41,7 +41,7 @@ func parseQueueAndWeight(queueAndWeight string) (queue string, weight int, err e parts := strings.Split(queueAndWeight, "=") // There must be exactly one '=' in queue/weight declaration if len(parts) > 2 { - return "", 0, errorNoneNumericWeight + return "", 0, errorNonNumericWeight } //The empty string is a valid queue name, and has the default weight of 1 @@ -62,7 +62,7 @@ func parseQueueAndWeight(queueAndWeight string) (queue string, weight int, err e queue = parts[0] weight, err = strconv.Atoi(parts[1]) if err != nil { - return "", 0, errorNoneNumericWeight + return "", 0, errorNonNumericWeight } return queue, weight, nil } From 0a3bfeb97940001e1d34f2209e039dfa0cf2e7e2 Mon Sep 17 00:00:00 2001 From: Benjamin Manns Date: Wed, 25 Sep 2013 00:24:02 -0400 Subject: [PATCH 06/16] Punctuate and format comments. --- process.go | 4 ++-- queues_flag.go | 13 ++++++------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/process.go b/process.go index b0c6fd3..2c593bf 100644 --- a/process.go +++ b/process.go @@ -76,12 +76,12 @@ func (p *process) fail(conn *redisConn) error { } func (p *process) queues(strict bool) []string { - //If the queues order is strict then just return them + // If the queues order is strict then just return them. if strict { return p.Queues } - //If not then we want to to shuffle the queues before returning them + // If not then we want to to shuffle the queues before returning them. queues := make([]string, len(p.Queues)) for i, v := range rand.Perm(len(p.Queues)) { queues[i] = p.Queues[v] diff --git a/queues_flag.go b/queues_flag.go index adcefe8..127c9bc 100644 --- a/queues_flag.go +++ b/queues_flag.go @@ -15,11 +15,11 @@ var ( type queuesFlag []string func (q *queuesFlag) Set(value string) error { - if value == "" { return errorEmptyQueues } - //Parse the individual queues and their weights if they are present. + + // Parse the individual queues and their weights if they are present. for _, queueAndWeight := range strings.Split(value, ",") { queue, weight, err := parseQueueAndWeight(queueAndWeight) if err != nil { @@ -39,17 +39,17 @@ func (q *queuesFlag) String() string { func parseQueueAndWeight(queueAndWeight string) (queue string, weight int, err error) { parts := strings.Split(queueAndWeight, "=") - // There must be exactly one '=' in queue/weight declaration + // There must be exactly one '=' in queue/weight declaration. if len(parts) > 2 { return "", 0, errorNonNumericWeight } - //The empty string is a valid queue name, and has the default weight of 1 + // The empty string is a valid queue name, and has the default weight of 1. if len(parts) == 0 { return "", 1, nil } - //If '=' is not present then we only have the queue name and the default weight is 1 + // If '=' is not present then we only have the queue name and the default weight is 1. if len(parts) == 1 { queue = parts[0] weight = 1 @@ -57,7 +57,7 @@ func parseQueueAndWeight(queueAndWeight string) (queue string, weight int, err e return } - //Check to see if we have a weight for this queue + // Check to see if we have a weight for this queue. if len(parts) == 2 { queue = parts[0] weight, err = strconv.Atoi(parts[1]) @@ -67,5 +67,4 @@ func parseQueueAndWeight(queueAndWeight string) (queue string, weight int, err e return queue, weight, nil } return - } From 7234027c395e8edff1614abf4a34e8227da9f273 Mon Sep 17 00:00:00 2001 From: Benjamin Manns Date: Wed, 25 Sep 2013 00:24:13 -0400 Subject: [PATCH 07/16] Add period to -queues flag documentation. --- README.md | 2 +- flags.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 26944b9..3217322 100644 --- a/README.md +++ b/README.md @@ -132,7 +132,7 @@ end There are several flags which control the operation of the goworker client. -* `-queues="comma,delimited,queues"` — This is the only required flag. The recommended practice is to separate your Resque workers from your goworkers with different queues. Otherwise, Resque worker classes that have no goworker analog will cause the goworker process to fail the jobs. Because of this, there is no default queue, nor is there a way to select all queues (à la Resque's `*` queue). If you have multiple queues you can assign them weights. A queue with a weight of 2 will be checked twice as often as a queue with a weight of 1: `-queues='high=2,low=1'` +* `-queues="comma,delimited,queues"` — This is the only required flag. The recommended practice is to separate your Resque workers from your goworkers with different queues. Otherwise, Resque worker classes that have no goworker analog will cause the goworker process to fail the jobs. Because of this, there is no default queue, nor is there a way to select all queues (à la Resque's `*` queue). If you have multiple queues you can assign them weights. A queue with a weight of 2 will be checked twice as often as a queue with a weight of 1: `-queues='high=2,low=1'`. * `-interval=5.0` — Specifies the wait period between polling if no job was in the queue the last time one was requested. * `-concurrency=25` — Specifies the number of concurrently executing workers. This number can be as low as 1 or rather comfortably as high as 100,000, and should be tuned to your workflow and the availability of outside resources. * `-connections=2` — Specifies the maximum number of Redis connections that goworker will consume between the poller and all workers. There is not much performance gain over two and a slight penalty when using only one. This is configurable in case you need to keep connection counts low for cloud Redis providers who limit plans on `maxclients`. diff --git a/flags.go b/flags.go index fc9a1b2..93ed84b 100644 --- a/flags.go +++ b/flags.go @@ -24,7 +24,7 @@ // If you have multiple queues you can assign // them weights. A queue with a weight of 2 will // be checked twice as often as a queue with a -// weight of 1: -queues='high=2,low=1' +// weight of 1: -queues='high=2,low=1'. // // -interval=5.0 // — Specifies the wait period between polling if From 8646e8b98344b4f28185b513c3e1d855cce066dd Mon Sep 17 00:00:00 2001 From: Benjamin Manns Date: Wed, 25 Sep 2013 00:42:17 -0400 Subject: [PATCH 08/16] We shouldn't permit blank queue entries. --- queues_flag_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/queues_flag_test.go b/queues_flag_test.go index e672230..62af68f 100644 --- a/queues_flag_test.go +++ b/queues_flag_test.go @@ -61,6 +61,16 @@ var queuesFlagSetTests = []struct { nil, errors.New("The weight must be a numeric value."), }, + { + "high=2,,,=1", + queuesFlag([]string{"high", "high"}), + nil, + }, + { + "=1", + nil, + errors.New("You must specify at least one queue."), + }, } func TestQueuesFlagSet(t *testing.T) { From e41fc484776b02442fd79c3ca05825fd9262c56d Mon Sep 17 00:00:00 2001 From: Benjamin Manns Date: Wed, 25 Sep 2013 00:42:47 -0400 Subject: [PATCH 09/16] Move empty flag check to queuesFlag.Set. --- queues_flag.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/queues_flag.go b/queues_flag.go index 127c9bc..ea2c937 100644 --- a/queues_flag.go +++ b/queues_flag.go @@ -21,6 +21,10 @@ func (q *queuesFlag) Set(value string) error { // Parse the individual queues and their weights if they are present. for _, queueAndWeight := range strings.Split(value, ",") { + if queueAndWeight == "" { + continue + } + queue, weight, err := parseQueueAndWeight(queueAndWeight) if err != nil { return err @@ -44,11 +48,6 @@ func parseQueueAndWeight(queueAndWeight string) (queue string, weight int, err e return "", 0, errorNonNumericWeight } - // The empty string is a valid queue name, and has the default weight of 1. - if len(parts) == 0 { - return "", 1, nil - } - // If '=' is not present then we only have the queue name and the default weight is 1. if len(parts) == 1 { queue = parts[0] From 9436daf6bbeee8ef1d747f12007283ff6182f1d3 Mon Sep 17 00:00:00 2001 From: Benjamin Manns Date: Wed, 25 Sep 2013 00:43:51 -0400 Subject: [PATCH 10/16] FIXUP empty test. --- queues_flag_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/queues_flag_test.go b/queues_flag_test.go index 62af68f..a8108e2 100644 --- a/queues_flag_test.go +++ b/queues_flag_test.go @@ -66,6 +66,11 @@ var queuesFlagSetTests = []struct { queuesFlag([]string{"high", "high"}), nil, }, + { + ",,,", + nil, + errors.New("You must specify at least one queue."), + }, { "=1", nil, From 83ab09f43417c9357ecef95c3f696a78c8f23133 Mon Sep 17 00:00:00 2001 From: Benjamin Manns Date: Wed, 25 Sep 2013 00:44:39 -0400 Subject: [PATCH 11/16] Use SplitN to ensure queueWeight size. --- queues_flag.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/queues_flag.go b/queues_flag.go index ea2c937..5af5178 100644 --- a/queues_flag.go +++ b/queues_flag.go @@ -42,11 +42,7 @@ func (q *queuesFlag) String() string { } func parseQueueAndWeight(queueAndWeight string) (queue string, weight int, err error) { - parts := strings.Split(queueAndWeight, "=") - // There must be exactly one '=' in queue/weight declaration. - if len(parts) > 2 { - return "", 0, errorNonNumericWeight - } + parts := strings.SplitN(queueAndWeight, "=", 2) // If '=' is not present then we only have the queue name and the default weight is 1. if len(parts) == 1 { From 53e7dc62483e28793767db7644951810e840abdd Mon Sep 17 00:00:00 2001 From: Benjamin Manns Date: Wed, 25 Sep 2013 00:50:22 -0400 Subject: [PATCH 12/16] Move empty queue check to after queue loop. --- queues_flag.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/queues_flag.go b/queues_flag.go index 5af5178..a2e2590 100644 --- a/queues_flag.go +++ b/queues_flag.go @@ -15,10 +15,6 @@ var ( type queuesFlag []string func (q *queuesFlag) Set(value string) error { - if value == "" { - return errorEmptyQueues - } - // Parse the individual queues and their weights if they are present. for _, queueAndWeight := range strings.Split(value, ",") { if queueAndWeight == "" { @@ -34,6 +30,9 @@ func (q *queuesFlag) Set(value string) error { *q = append(*q, queue) } } + if len(*q) == 0 { + return errorEmptyQueues + } return nil } From a5d50248c6d5cc14ccfa1cafab9b67a13e0760d9 Mon Sep 17 00:00:00 2001 From: Benjamin Manns Date: Wed, 25 Sep 2013 00:50:51 -0400 Subject: [PATCH 13/16] Set queue early in parseQueueAndWeight. --- queues_flag.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/queues_flag.go b/queues_flag.go index a2e2590..cc1c20e 100644 --- a/queues_flag.go +++ b/queues_flag.go @@ -42,10 +42,10 @@ func (q *queuesFlag) String() string { func parseQueueAndWeight(queueAndWeight string) (queue string, weight int, err error) { parts := strings.SplitN(queueAndWeight, "=", 2) + queue = parts[0] // If '=' is not present then we only have the queue name and the default weight is 1. if len(parts) == 1 { - queue = parts[0] weight = 1 err = nil return @@ -53,7 +53,6 @@ func parseQueueAndWeight(queueAndWeight string) (queue string, weight int, err e // Check to see if we have a weight for this queue. if len(parts) == 2 { - queue = parts[0] weight, err = strconv.Atoi(parts[1]) if err != nil { return "", 0, errorNonNumericWeight From f152ef0392ef4ecf8e2e8083d7eef1cf85d7cc4a Mon Sep 17 00:00:00 2001 From: Benjamin Manns Date: Wed, 25 Sep 2013 00:51:54 -0400 Subject: [PATCH 14/16] Return (with weight=0) if queue is empty. --- queues_flag.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/queues_flag.go b/queues_flag.go index cc1c20e..f308876 100644 --- a/queues_flag.go +++ b/queues_flag.go @@ -44,6 +44,10 @@ func parseQueueAndWeight(queueAndWeight string) (queue string, weight int, err e parts := strings.SplitN(queueAndWeight, "=", 2) queue = parts[0] + if queue == "" { + return + } + // If '=' is not present then we only have the queue name and the default weight is 1. if len(parts) == 1 { weight = 1 From 3b04f5d32be5ff6e676caa5f4a386997e93c8a51 Mon Sep 17 00:00:00 2001 From: Benjamin Manns Date: Wed, 25 Sep 2013 00:52:25 -0400 Subject: [PATCH 15/16] Simplify parseQueueAndWeight if block. --- queues_flag.go | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/queues_flag.go b/queues_flag.go index f308876..2c91e40 100644 --- a/queues_flag.go +++ b/queues_flag.go @@ -48,20 +48,13 @@ func parseQueueAndWeight(queueAndWeight string) (queue string, weight int, err e return } - // If '=' is not present then we only have the queue name and the default weight is 1. if len(parts) == 1 { weight = 1 - err = nil - return - } - - // Check to see if we have a weight for this queue. - if len(parts) == 2 { + } else { weight, err = strconv.Atoi(parts[1]) if err != nil { - return "", 0, errorNonNumericWeight + err = errorNonNumericWeight } - return queue, weight, nil } return } From 244ad15fa6e6ee8ee3ba35197661a394ac1fbb6c Mon Sep 17 00:00:00 2001 From: Benjamin Manns Date: Wed, 25 Sep 2013 00:52:46 -0400 Subject: [PATCH 16/16] Remove explanatory comment from queuesFlag.Set. --- queues_flag.go | 1 - 1 file changed, 1 deletion(-) diff --git a/queues_flag.go b/queues_flag.go index 2c91e40..e5afac8 100644 --- a/queues_flag.go +++ b/queues_flag.go @@ -15,7 +15,6 @@ var ( type queuesFlag []string func (q *queuesFlag) Set(value string) error { - // Parse the individual queues and their weights if they are present. for _, queueAndWeight := range strings.Split(value, ",") { if queueAndWeight == "" { continue