Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

New scripts to fetch github data and update pluginsdb. #137

Open
wants to merge 8 commits into from

2 participants

@aulvi
Owner

update-plugin-stats.js is the main entry point, this should be called via cron.

github-stats.js simply takes a Plugins object and returns an updated Plugins object with new 'watchers' and 'forks' data.

scripts/github-stats.js
@@ -0,0 +1,89 @@
+/* jshint laxcomma: true */
@scottgonzalez Owner

No one-off rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
scripts/github-stats.js
@@ -0,0 +1,89 @@
+/* jshint laxcomma: true */
+var
+ https = require("https")
+ , util = require("util")
@scottgonzalez Owner

Never comma first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
scripts/github-stats.js
@@ -0,0 +1,89 @@
+/* jshint laxcomma: true */
+var
+ https = require("https")
@scottgonzalez Owner

spacing: require( "https" )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
scripts/github-stats.js
@@ -0,0 +1,89 @@
+/* jshint laxcomma: true */
+var
+ https = require("https")
+ , util = require("util")
+;
@scottgonzalez Owner

Comma on same line as code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
scripts/github-stats.js
@@ -0,0 +1,89 @@
+/* jshint laxcomma: true */
+var
+ https = require("https")
+ , util = require("util")
+;
+
+module.exports = function(plugin, callback) {
@scottgonzalez Owner

spacing: function( plugin, callback )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
scripts/update-plugin-stats.js
@@ -0,0 +1,60 @@
+/**
@scottgonzalez Owner

No file name comments like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
scripts/update-plugin-stats.js
@@ -0,0 +1,60 @@
+/**
+ * update-plugin-stats.js
+ */
+/* jshint laxcomma: true */
+var db = require("../lib/pluginsdb.js")
@scottgonzalez Owner

Never specify the extension for a module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
scripts/update-plugin-stats.js
@@ -0,0 +1,60 @@
+/**
+ * update-plugin-stats.js
+ */
+/* jshint laxcomma: true */
+var db = require("../lib/pluginsdb.js")
+ , util = require("util")
+ , getstats = require("./github-stats.js")
+ , delay = 1000
+ , queue
@scottgonzalez Owner

Undefined vars go at the top.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
scripts/update-plugin-stats.js
@@ -0,0 +1,60 @@
+/**
+ * update-plugin-stats.js
+ */
+/* jshint laxcomma: true */
+var db = require("../lib/pluginsdb.js")
+ , util = require("util")
+ , getstats = require("./github-stats.js")
+ , delay = 1000
+ , queue
+ , iv
+ , ready
+;
+
+// Very simple logger
@scottgonzalez Owner

We have a logger. Check the other scripts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
scripts/update-plugin-stats.js
((8 lines not shown))
+ , delay = 1000
+ , queue
+ , iv
+ , ready
+;
+
+// Very simple logger
+function logger(msg) {
+ if (msg) {
+ console.log("[Logger] " + msg);
+ }
+}
+
+function update(err, plugin) {
+ if (err) {
+ logger(err);
@scottgonzalez Owner

Always return on error. The main body of a function shouldn't be inside a conditional.

if ( error ) {
    logger( error );
    return;
}

db.updateRepoMeta( ... );
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
scripts/update-plugin-stats.js
((39 lines not shown))
+ }
+}
+
+function init(err, plugins) {
+ if (!err && plugins.length > 0) {
+ queue = plugins;
+ ready = true;
+ iv = setInterval(processData, delay);
+ }
+}
+
+// Kick off
+db.getAllPlugins(init);
+
+// Let the current action finish, then stop processing and exit
+function shutdownHook() {
@scottgonzalez Owner

Cron jobs don't need shutdown hooks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
scripts/github-stats.js
((12 lines not shown))
+
+ if (plugin.repo) {
+ repoPath = plugin.repo.split("/").slice(1).join("/");
+ }
+
+ if (!repoPath || repoPath.length === 0) {
+ if (typeof(callback) === "function") {
+ callback("Invalid repository");
+ }
+ }
+
+ options = {
+ host: "api.github.com",
+ path: "/repos/" + repoPath,
+ headers: {
+ "user-agent": "node.js scraper"
@scottgonzalez Owner

This is super vague and defeats the purpose of GitHub requiring a UA string.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
scripts/github-stats.js
((2 lines not shown))
+var
+ https = require("https")
+ , util = require("util")
+;
+
+module.exports = function(plugin, callback) {
+ function buildOptions() {
+ var options
+ , repoPath = null
+ ;
+
+ if (plugin.repo) {
+ repoPath = plugin.repo.split("/").slice(1).join("/");
+ }
+
+ if (!repoPath || repoPath.length === 0) {
@scottgonzalez Owner

Let's assume that we're always getting valid data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
scripts/github-stats.js
((47 lines not shown))
+
+ res.on("end", function () {
+ switch (res.statusCode) {
+
+ case 304:
+ console.log("304, no work to do");
+ break;
+
+ case 200:
+ try {
+ data = JSON.parse(data);
+ plugin.watchers = data.watchers_count;
+ plugin.forks = data.forks_count;
+ plugin.etag = res.headers.etag;
+
+ console.log("Plugin " + plugin.plugin + " updated!");
@scottgonzalez Owner

Use the logger.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
scripts/github-stats.js
((60 lines not shown))
+ plugin.etag = res.headers.etag;
+
+ console.log("Plugin " + plugin.plugin + " updated!");
+ console.log("Watchers: " + data.watchers_count + " Forks: " + data.forks_count + "\n");
+ } catch (err) {
+ error = err;
+ }
+ break;
+
+ default:
+ // Well that's weird, wat do?
+ console.log("Unexpected reply, take a look:\n\n" + data);
+ break;
+ }
+
+ if (typeof(callback) === "function") {
@scottgonzalez Owner

Callback should just be required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
lib/pluginsdb.js
@@ -141,6 +141,18 @@ var pluginsDb = module.exports = {
});
}),
+ getAllPlugins: auto(function( fn ) {
+ db.all( "SELECT * FROM plugins", function( error, rows ) {
+ if ( error ) {
+ return fn( error );
+ }
+
+ fn( null, rows.map(function( row ) {
@scottgonzalez Owner

Why does this need to be mapped?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
scripts/github-stats.js
((61 lines not shown))
+
+ console.log("Plugin " + plugin.plugin + " updated!");
+ console.log("Watchers: " + data.watchers_count + " Forks: " + data.forks_count + "\n");
+ } catch (err) {
+ error = err;
+ }
+ break;
+
+ default:
+ // Well that's weird, wat do?
+ console.log("Unexpected reply, take a look:\n\n" + data);
+ break;
+ }
+
+ if (typeof(callback) === "function") {
+ callback(error, plugin);
@scottgonzalez Owner

Never pass both pieces of data. It's either an error with no data, or it's successful with an explicit null for the error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
scripts/github-stats.js
((50 lines not shown))
+
+ case 304:
+ console.log("304, no work to do");
+ break;
+
+ case 200:
+ try {
+ data = JSON.parse(data);
+ plugin.watchers = data.watchers_count;
+ plugin.forks = data.forks_count;
+ plugin.etag = res.headers.etag;
+
+ console.log("Plugin " + plugin.plugin + " updated!");
+ console.log("Watchers: " + data.watchers_count + " Forks: " + data.forks_count + "\n");
+ } catch (err) {
+ error = err;
@scottgonzalez Owner

Don't assign to a "master" error, just invoke the callback and return.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
scripts/update-plugin-stats.js
((31 lines not shown))
+
+ if (ready && data) {
+ getstats(data, update);
+ } else {
+ if(!ready || iv) {
+ clearInterval(iv);
+ process.exit(0);
+ }
+ }
+}
+
+function init(err, plugins) {
+ if (!err && plugins.length > 0) {
+ queue = plugins;
+ ready = true;
+ iv = setInterval(processData, delay);
@scottgonzalez Owner

Never use an interval to process data. Any delays that slow down the process will cause massive build up and kill the server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@scottgonzalez

You should read our style guide.

The github-stats.js file doesn't belong in the scripts directory, since it has exports and won't do anything if run directly. This logic should just be implemented in lib/service/github.js.

scripts/github-stats.js
((34 lines not shown))
+ }
+
+ return options;
+ }
+
+ function response(res) {
+ var data = ""
+ , error = null
+ ;
+
+ res.on("data", function (chunk) {
+ data += chunk;
+ });
+
+ res.on("end", function () {
+ switch (res.statusCode) {
@scottgonzalez Owner

No switch statements unless you're using fall-through. Switch to if.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@scottgonzalez

Based on discussion in IRC just now, we'll use a long running script instead of a cron job. We'll process one plugin per minute, which should allow us to process all plugins in one day right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
3  config-sample.json
@@ -5,5 +5,8 @@
"url": "local.plugins.jquery.com",
"username": "admin",
"password": "secret"
+ },
+ "github": {
+ "token": "oauthtoken"
}
}
View
10 lib/pluginsdb.js
@@ -141,6 +141,16 @@ var pluginsDb = module.exports = {
});
}),
+ getAllPlugins: auto(function( fn ) {
+ db.all( "SELECT * FROM plugins", function( error, rows ) {
+ if ( error ) {
+ return fn( error );
+ }
+
+ fn( null, rows );
+ });
+ }),
+
_setup: function( fn ) {
var Step = require( "step" );
View
47 lib/service/github.js
@@ -3,7 +3,10 @@ var fs = require( "fs" ),
exec = require( "child_process" ).exec,
Step = require( "step" ),
mkdirp = require( "mkdirp" ),
- service = require( "../service" );
+ service = require( "../service" ),
+ logger = require( "../logger" ),
+ config = require( "../config" ),
+ https = require( "https" );
var reGithubUrl = /^https?:\/\/github\.com\/([^\/]+)\/([^\/]+)(\/.*)?$/;
@@ -65,6 +68,48 @@ extend( GithubRepo.prototype, {
return this.siteUrl + "/zipball/" + version;
},
+ getStats: function( plugin, fn ) {
+
+ function response( res ) {
+ var data = "";
+
+ res.on( "data", function(chunk){ data += chunk; } );
+
+ res.on( "end", function(){
+
+ if ( res.statusCode === 200 ) {
+ try {
+ data = JSON.parse( data );
+ plugin.watchers = data.watchers_count;
+ plugin.forks = data.forks_count;
+ logger.log(
+ "Updated " + plugin.plugin +
+ " Watchers(" + plugin.watchers + ")" +
+ " Forks(" + plugin.forks + ")"
+ );
+ } catch ( error ) {
+ return fn( error );
+ }
+ } else {
+ logger.error( "Unexpected reply: " + data );
+ }
+
+ fn( null, plugin );
+ });
+ }
+
+ var options = {
+ host: "api.github.com",
+ path: "/repos/" + plugin.repo.split( "/" ).slice( 1 ).join( "/" ),
+ headers: {
+ "user-agent": "stats/0.1 (+http://plugins.jquery.com)",
+ "Authorization": "token " + config.github.token
+ }
+ };
+
+ https.request( options, response ).on( "error", logger.error ).end();
+ },
+
getTags: function( fn ) {
var repo = this;
Step(
View
59 scripts/update-plugin-stats.js
@@ -0,0 +1,59 @@
+var db = require( "../lib/pluginsdb" ),
+ service = require( "../lib/service" ),
+ logger = require( "../lib/logger" ),
+ iv,
+ queue,
+ ready,
+ delay = 60*1000;
+
+function update( err, plugin ) {
+ if ( err ) {
+ logger.log( err );
+ } else {
+ db.updateRepoMeta( plugin.repo, plugin, function(err){
+ if ( err ) {
+ logger.log( err );
+ }
+ iv = setTimeout( processData, delay );
+ });
+ }
+}
+
+function processData() {
+ var repo,
+ data = queue.shift();
+
+ if ( !ready ) {
+ clearTimeout( iv );
+ process.exit( 0 );
+ } else if ( !data ) {
+ iv = setTimeout( init, delay );
+ } else if ( ready && data ) {
+ repo = service.getRepoById( data.repo );
+ repo.getStats( data, update );
+ }
+}
+
+function init() {
+ db.getAllPlugins(function( err, plugins ) {
+ if ( !err && plugins.length > 0 ) {
+ queue = plugins;
+ ready = true;
+ processData();
+ } else {
+ iv = setTimeout( init, delay );
+ }
+ });
+}
+
+// Kick off
+init();
+
+// Let the current action finish, then stop processing and exit
+function shutdownHook() {
+ logger.log( "Shutting down update-plugin-stats." );
+ ready = false;
+}
+
+process.once( "SIGINT", shutdownHook );
+process.once( "SIGTERM", shutdownHook );
Something went wrong with that request. Please try again.