Skip to content

Commit

Permalink
Fix #104: Manage, throttle comments with Redis
Browse files Browse the repository at this point in the history
  • Loading branch information
openjck committed Jul 31, 2015
1 parent dcabc55 commit 6d3d270
Show file tree
Hide file tree
Showing 9 changed files with 112 additions and 13 deletions.
3 changes: 3 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,6 @@ language: node_js

node_js:
- "0.12"

services:
- redis-server
25 changes: 20 additions & 5 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,24 @@ GitHub.

# Submitting a pull request

Before submitting a pull request, run the following commands to test your
changes and spruce things up.
Before submitting a pull request, follow these steps to spruce things up and
test your changes.

1. `npm install`
2. `./gulp test`
3. `./gulp beautify`
## Setup

To get started, install all dependencies and set up Redis:

1. Run `npm install`
2. Follow [these steps](http://redis.io/topics/quickstart#installing-redis)
to install a local Redis server

## Beautify your code

Run this command to format your code according to our conventions:

`./gulp beautify`

## Run tests

1. Start the local Redis server
2. Run `./gulp test`
2 changes: 2 additions & 0 deletions Procfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
web: node bin/www.js
worker: node worker.js
9 changes: 6 additions & 3 deletions commenter.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ var logger = require('./logger');
* instance of review_comments_url.
* https://developer.github.com/v3/activity/events/types/#pullrequestevent
*/
function postPullRequestComment(commentURL, comment, filename, commitSHA, line) {

function postPullRequestComment(commentURL, comment, filename, commitSHA, line, done) {
sendRequest({
url: commentURL,
method: 'POST',
Expand All @@ -31,7 +30,11 @@ function postPullRequestComment(commentURL, comment, filename, commitSHA, line)
position: line
})
}, function(error) {
if (error) return logger.error('Error posting pull request comment to ' + commentURL + ' regarding commit ' + commitSHA + ':', error);
if (error) {
logger.error('Error posting pull request comment to ' + commentURL + ' regarding commit ' + commitSHA + ':', error);
} else {
done(); // Mark the Redis job as done
}
});
}

Expand Down
19 changes: 17 additions & 2 deletions hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
var github = require('octonode');
var Q = require('q');

var commenter = require('./commenter');
var diff = require('./diffParse');
var logger = require('./logger');
var processor = require('./processor');
var utils = require('./utils');
var config = require('./config');
var redisQueue = require('./redisQueue');

var configFilename = '.doiuse';
var githubClient = github.client(config.token);
Expand Down Expand Up @@ -61,7 +61,22 @@ function processPullRequest(destinationRepo, originRepo, originBranch, prNumber,
// Callback for handling an incompatible line of code
var line = diff.lineToIndex(file.patch, incompatibility.usage.source.start.line);
var comment = incompatibility.featureData.title + ' not supported by: ' + incompatibility.featureData.missing;
commenter.postPullRequestComment(commentURL, comment, file.filename, currentCommit.sha, line);

// Store a comment task in Redis for later processing
var commentJob = redisQueue.create('comment', {
commentURL: commentURL,
sha: currentCommit.sha,
filename: file.filename,
line: line,
comment: comment
});
commentJob.attempts(process.env.COMMENT_ATTEMPTS);
commentJob.backoff({
type: 'exponential'
});
commentJob.save(function(error) {
if (error) return logger.error(error);
});
});

});
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
"main": "app.js",
"private": true,
"scripts": {
"start": "node ./bin/www",
"postinstall": "./gulp --gulpfile gulpfile-prod.js build",
"test": "./gulp test"
},
Expand Down Expand Up @@ -34,6 +33,7 @@
"express": "^4.8.6",
"gulp": "^3.8.11",
"gulp-marked": "^1.0.0",
"kue": "^0.9.4",
"newrelic": "^1.21.1",
"nunjucks": "^1.3.4",
"octonode": "^0.7.1",
Expand Down
32 changes: 32 additions & 0 deletions redisQueue.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/**
* Return an instance of the Kue queue singleton.
*
* This file is only needed to work around bug #681 of Kue:
* https://github.com/Automattic/kue/issues/681
*
* Once the bug is fixed, we'll be able to get the queue with much less code,
* making this file unnecessary:
*
* var kue = require('kue');
* var redisQueue = kue.createQueue({
* redis: process.env.REDIS_URL
* });
*/

var kue = require('kue');
var url = require('url');

var parsedRedisURL = url.parse(process.env.REDIS_URL);

var config = {
redis: {
port: parseInt(parsedRedisURL.port),
host: parsedRedisURL.hostname
}
};

if (parsedRedisURL.auth) {
config.redis.auth = parsedRedisURL.auth.replace(/.*?:/, '');
}

module.exports = kue.createQueue(config);
9 changes: 7 additions & 2 deletions tests/tests.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,20 @@
'use strict';

// Set environment variables that will be read during testing. These values will
// be read in config.js, so they need to be set before the file is loaded.
// Set environment variables that will be needed during testing. These values
// will be read in config.js, so they need to be set before the file is loaded.
var testedTrackingID = '654321';
process.env.TRACKING_ID = testedTrackingID;
process.env.REDIS_URL = 'redis://localhost';
process.env.COMMENT_WAIT = 0; // The comment wait is only needed in production to avoid tripping a GitHub spam protection system

// Pretend that we're running in production mode so that we can test
// production-only features like Google Analytics tracking. This value is read
// in app.js, so it needs to be set before the file is loaded.
process.env.NODE_ENV = 'production';

// Process Redis tasks
require('../worker');

var fs = require('fs');
var path = require('path');

Expand Down
24 changes: 24 additions & 0 deletions worker.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
'use strict';

var commenter = require('./commenter');
var redisQueue = require('./redisQueue');

var lastCommentTimestamp = 0;

// Process comment tasks
// Pause for COMMENT_WAIT milleseconds between comment submissions
redisQueue.process('comment', function(job, done) {
var now = Date.now();
var waitRemaining = 0;

var nextCommentTimestamp = lastCommentTimestamp + parseInt(process.env.COMMENT_WAIT);

if (nextCommentTimestamp > now) {
waitRemaining = nextCommentTimestamp - now;
}

setTimeout(function() {
commenter.postPullRequestComment(job.data.commentURL, job.data.comment, job.data.filename, job.data.sha, job.data.line, done);
lastCommentTimestamp = Date.now(); // Use a new, fresh value of Date.now()
}, waitRemaining);
});

0 comments on commit 6d3d270

Please sign in to comment.