update readme and add env defs

Runnable · Sep 6, 2016 · 9a11902 · 9a11902
1 parent 490d305
commit 9a11902
Show file tree

Hide file tree

Showing 6 changed files with 90 additions and 42 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -8,11 +8,14 @@ cache:
 sudo: false
 services:
 - rabbitmq
+- redis
 env:
   global:
   - LOG_LEVEL=error
   - RABBITMQ_USERNAME=guest
   - RABBITMQ_PASSWORD=guest
+  - REDIS_HOST=localhost
+  - REDIS_PORT=6379
 script:
 - npm run lint
 - npm run coverage

diff --git a/README.md b/README.md
@@ -14,14 +14,17 @@ An opinionated queue based worker server for node.
 
 For ease of use we provide options to set the host, port, username, and password to the RabbitMQ server. If not present in options, the server will attempt to use the following environment variables and final defaults:
 
-options                  | environment         | default
--------------------------|---------------------|--------------
-`opts.rabbitmq.hostname` | `RABBITMQ_HOSTNAME` | `'localhost'`
-`opts.rabbitmq.port`     | `RABBITMQ_PORT`     | `'5672'`
-`opts.rabbitmq.username` | `RABBITMQ_USERNAME` | _none_
-`opts.rabbitmq.password` | `RABBITMQ_PASSWORD` | _none_
-`opts.log`               | _N/A_               | Basic [bunyan](https://github.com/trentm/node-bunyan) instance with `stdout` stream (for logging)
-`opts.errorCat`          | _N/A_               | Basic [error-cat](https://github.com/runnable/error-cat) instance (for rollbar error reporting)
+options                             | environment                 | default
+------------------------------------|-----------------------------|--------------
+`opts.rabbitmq.hostname`            | `RABBITMQ_HOSTNAME`         | `'localhost'`
+`opts.rabbitmq.port`                | `RABBITMQ_PORT`             | `'5672'`
+`opts.rabbitmq.username`            | `RABBITMQ_USERNAME`         | _none_
+`opts.rabbitmq.password`            | `RABBITMQ_PASSWORD`         | _none_
+`opts.redisRateLimiting.host`       | `REDIS_HOST`                | `'localhost'`
+`opts.redisRateLimiting.port`       | `REDIS_PORT`                | `'6379'`
+`opts.redisRateLimiting.durationMs` | `RATE_LIMIT_DURATION`       | `1000`
+`opts.log`                          | _N/A_                       | Basic [bunyan](https://github.com/trentm/node-bunyan) instance with `stdout` stream (for logging)
+`opts.errorCat`                     | _N/A_                       | Basic [error-cat](https://github.com/runnable/error-cat) instance (for rollbar error reporting)
 
 Other options for Ponos are as follows:
 
@@ -141,11 +144,26 @@ server.setAllTasks({
   'queue-1': queueOneTaskFn,
   // This will use the specified timeout, maxNumRetries, ...
   'queue-2': {
+    // worker function to run
     task: queueTwoTaskFn,
+
+    // schema to validate job against
     jobSchema: Joi.object({ tid: Joi.string() }),
+
+    // time before job will throw timeout error
     msTimeout: 1337,
+
+    // number of times before job will stop retrying on failure
     maxNumRetries: 7,
-    finalRetryFn: () => { return Promise.try(...)}
+
+    // function to run when we hit max retries
+    finalRetryFn: () => { return Promise.try(...)},
+
+    // number of jobs that we can perform in given duration
+    maxOperations: 5,
+
+    // duration under which rate limit is accounted for
+    durationMs: 60000
   }
 })
 ```

diff --git a/package.json b/package.json
@@ -17,6 +17,7 @@
     "docs": "npm run build && jsdoc --recurse --readme ./README.md lib/",
     "format": "standard --format",
     "functional": "mocha $npm_package_options_mocha test/functional",
+    "integration": "mocha $npm_package_options_mocha test/integration",
     "lint": "npm run lint:format && npm run lint:type",
     "lint:format": "standard --verbose",
     "lint:type": "flow --timeout 30",
@@ -40,6 +41,7 @@
   ],
   "author": "Ryan Sandor Richards <sandor.richards@gmail.com>",
   "contributors": [
+    "Anandkumar patel <anand@runnable.com>",
     "Bryan Kendall <bryan@runnable.com>"
   ],
   "license": "MIT",

diff --git a/src/rate-limiters/redis.js b/src/rate-limiters/redis.js
@@ -29,11 +29,20 @@ module.exports = class RedisRateLimiter {
    * @return {RedisRateLimiter}
    */
   constructor (opts: Object) {
-    this.port = opts.port
-    this.host = opts.host
+    this.port = opts.port ||
+      process.env.REDIS_PORT ||
+      '6379'
+
+    this.host = opts.host ||
+      process.env.REDIS_HOST ||
+      'localhost'
+
+    this.durationMs = opts.durationMs ||
+      parseInt(process.env.RATE_LIMIT_DURATION, 10) ||
+      1000
+
     this.log = opts.log
-    // default to 1 second
-    this.durationMs = opts.durationMs || 1000
+
     joi.assert(this, optsSchema)
   }
 

diff --git a/test/integration/rate-limiters/redis.js b/test/integration/rate-limiters/redis.js
@@ -1,7 +1,6 @@
 'use strict'
 const chai = require('chai')
 const Promise = require('bluebird')
-const put = require('101/put')
 const redis = require('redis')
 const sinon = require('sinon')
 
@@ -10,18 +9,14 @@ const RedisRateLimiter = require('../../../src/rate-limiters/redis')
 
 const assert = chai.assert
 
-const redisConfig = {
-  port: '6379',
-  host: 'localhost'
-}
 describe('rate-limiters/redis integration test', () => {
   let redisRateLimiter
-  const testClient = redis.createClient(redisConfig.port, redisConfig.host)
+  const testClient = redis.createClient('6379', 'localhost')
 
   beforeEach((done) => {
-    redisRateLimiter = new RedisRateLimiter(put(redisConfig, {
+    redisRateLimiter = new RedisRateLimiter({
       log: logger.child({ module: 'ponos:test' })
-    }))
+    })
     sinon.spy(Promise, 'delay')
     redisRateLimiter.connect()
       .then(() => {

diff --git a/test/unit/rate-limiters/redis.js b/test/unit/rate-limiters/redis.js
@@ -16,47 +16,68 @@ describe('redis', () => {
   let testRedisRateLimiter
 
   beforeEach(() => {
+    delete process.env.REDIS_PORT
+    delete process.env.REDIS_HOST
+    delete process.env.RATE_LIMIT_DURATION
+
     testOpts = {
-      port: '123',
-      host: 'localhost',
+      port: '1',
+      host: 'remotehost',
       log: logger
     }
     testRedisRateLimiter = new RedisRateLimiter(testOpts)
   })
 
   describe('constructor', () => {
-    let out
-    it('should throw validation error if missing port', () => {
+    it('should use passed port', () => {
+      const out = new RedisRateLimiter(testOpts)
+      assert.equal(out.port, testOpts.port)
+    })
+
+    it('should use env port', () => {
+      delete testOpts.port
+      process.env.REDIS_PORT = '1234'
+      const out = new RedisRateLimiter(testOpts)
+      assert.equal(out.port, process.env.REDIS_PORT)
+    })
+
+    it('should use default port', () => {
       delete testOpts.port
-      assert.throws(() => {
-        out = new RedisRateLimiter(testOpts)
-      }, Error, /"port" is required/)
-      assert.isUndefined(out)
+      const out = new RedisRateLimiter(testOpts)
+      assert.equal(out.port, '6379')
     })
 
-    it('should throw validation error if missing host', () => {
+    it('should use passed host', () => {
+      const out = new RedisRateLimiter(testOpts)
+      assert.equal(out.host, testOpts.host)
+    })
+
+    it('should use env host', () => {
       delete testOpts.host
-      assert.throws(() => {
-        out = new RedisRateLimiter(testOpts)
-      }, Error, /"host" is required/)
-      assert.isUndefined(out)
+      process.env.REDIS_HOST = 'moonhost'
+      const out = new RedisRateLimiter(testOpts)
+      assert.equal(out.host, process.env.REDIS_HOST)
     })
 
-    it('should throw validation error if missing log', () => {
-      delete testOpts.log
-      assert.throws(() => {
-        out = new RedisRateLimiter(testOpts)
-      }, Error, /"log" is required/)
-      assert.isUndefined(out)
+    it('should use default host', () => {
+      delete testOpts.host
+      const out = new RedisRateLimiter(testOpts)
+      assert.equal(out.host, 'localhost')
     })
 
     it('should default durationMs to 1000', () => {
-      out = new RedisRateLimiter(testOpts)
+      const out = new RedisRateLimiter(testOpts)
       assert.equal(out.durationMs, 1000)
     })
 
+    it('should use env durationMs', () => {
+      process.env.RATE_LIMIT_DURATION = '5678'
+      const out = new RedisRateLimiter(testOpts)
+      assert.equal(out.durationMs, process.env.RATE_LIMIT_DURATION)
+    })
+
     it('should use passed durationMs', () => {
-      out = new RedisRateLimiter(put({
+      const out = new RedisRateLimiter(put({
         durationMs: 1738
       }, testOpts))
       assert.equal(out.durationMs, 1738)