probot / github-app

node module to handle authentication for the GitHub Apps API
51 stars 19 forks source link

Update github #8

Closed hiimbex closed 7 years ago

hiimbex commented 7 years ago

There was previously no support in node-github for has_projects. According to the maintainer here, it's available in the latest npm version, so this updates github to 9.3.1.

This will close https://github.com/probot/settings/issues/34

hiimbex commented 7 years ago

@bkeepers Does this require a new version of github-app to be released as well?

bkeepers commented 7 years ago

@bkeepers Does this require a new version of github-app to be released as well?

Yeah 😢

It'd be great to figure out how to avoid depending on a specific version of node-github. peerDependencies would be one way of doing it, but it would require the caller of this code to pass in an instance of the GitHub client.

bkeepers commented 7 years ago

3.1.0 is published

jeffwilcox commented 7 years ago

@bkeepers I'd be fine if github-app depended on whatever version of node-github that you like, as long as I could provide my own instance of either the required library or GitHubApi.

Which sort of syntax or approach would you be more of a fan of?

const GitHubApi = require('github');
const github = new GitHubApi();
const createApp = require('github-app');
const app = createApp({id: 987, cert: ..., github: github)

By providing an instance, there are more opportunities to monkey patch, etc.

Otherwise, if you wanted to make sure the debug flag could still work, it would be:

const GitHubApi = require('github');
const createApp = require('github-app');
const app = createApp({id: 987, cert: ..., github: GitHubApi)

Thoughts?

bkeepers commented 7 years ago

@jeffwilcox yeah, I'd be fine with either of those.

Ideally we'd just pull this functionality into node-github:

const GitHubApi = require('github');
const github = new GitHubApi();

github.authenticate({
    type: "app",
    id: process.env.APP_ID,
    cert: process.env.PRIVATE_KEY,
});
jeffwilcox commented 7 years ago

Good point... though I'm also starting to think about the right way to get this working in long-lived apps. I prefer to share a single instance of github in my app, but it may run for hours/days.

bkeepers commented 7 years ago

Ahh yeah, that's interesting. For app authentication, I could imagine the example above would actually generate a new jwt for each request.

The bigger issue will probably be installation tokens, which need to be created through the API and only last 60 minutes. In Probot (which is mostly event driven, so may not be a great example for a long-lived app) there is an auth method that caches tokens for 59 minutes, so it assumes anything that requests a client will finish within a minute.

What about adding support for installation tokens to node-github and teach it to fetch a new token if the current one is expired? I don't know exactly what the implementation would look like.