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 bdc279a
Show file tree
Hide file tree
Showing 10 changed files with 132 additions and 16 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
30 changes: 24 additions & 6 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ the contribution workflow at this point.
create a token
4. From the terminal, add your newly created GitHub personal access token to the
Heroku config: heroku config:set OAUTH_TOKEN=#######
5. Create a test repository which you'll send invalid CSS or Stylus files to,
5. Enable the Heroku Redis add-on (you may need to enter a credit card for
verification): `heroku addons:create heroku-redis:hobby-dev`
6. Start a worker: `heroku ps:scale worker=1`
7. Create a test repository which you'll send invalid CSS or Stylus files to,
which your Discord instance will evaluate. Within that repository:
1. Navigate to *Settings* > *Webhooks and Services*
2. Add a webhook
Expand All @@ -27,9 +30,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
14 changes: 9 additions & 5 deletions commenter.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,13 @@
var sendRequest = require('request');

var config = require('./config');
var logger = require('./logger');

/**
* Post a line comment on a particular pull request. commentURL should be an
* 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 @@ -30,8 +28,14 @@ function postPullRequestComment(commentURL, comment, filename, commitSHA, line)
commit_id: commitSHA,
position: line
})
}, function(error) {
if (error) return logger.error('Error posting pull request comment to ' + commentURL + ' regarding commit ' + commitSHA + ':', error);
}, function(error, response) {
// If the comment could not be submitted, notify Redis so that the job
// can be re-attempted later. Otherwise, mark the job as done.
if(error || response.statusCode === 403) {
return done(new Error('Comment could not be submitted'));
} else {
done();
}
});
}

Expand Down
2 changes: 2 additions & 0 deletions config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ var config = {
brand: 'Discord',
token: process.env.OAUTH_TOKEN,
trackingID: process.env.TRACKING_ID,
commentWait: process.env.COMMENT_WAIT || 250,
commentAttempts: process.env.COMMENT_ATTEMPTS || 10,

// Used when no environment variable (process.env.PORT) is set
port: 3000,
Expand Down
29 changes: 27 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,32 @@ 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);

// Create a Redis job that will submit the comment
var commentJob = redisQueue.create('comment', {
commentURL: commentURL,
sha: currentCommit.sha,
filename: file.filename,
line: line,
comment: comment
});

// If the comment is rejected, re-attempt several times with
// exponentially longer waits between each attempt.
commentJob.attempts(config.commentAttempts);
commentJob.backoff({
type: 'exponential'
});

// If the comment is rejected after several attempts, log an
// error.
commentJob.on('failed', function() {
logger.error('Error posting comment to line ' + line + ' of ' + file.filename + ' in ' + originRepo + ' pull request #' + prNumber);
});

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 bugfix is shipped, 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
25 changes: 25 additions & 0 deletions worker.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
'use strict';

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

var lastCommentTimestamp = 0;

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

var nextCommentTimestamp = lastCommentTimestamp + parseInt(config.commentWait);

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 timestamp
}, waitRemaining);
});

0 comments on commit bdc279a

Please sign in to comment.