Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #104: Manage, throttle comments with Redis #131

Merged
merged 1 commit into from
Aug 4, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
});