Skip to content

Added GHE enterprise connection option #25

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.

Sign up for GitHub

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

Open
nikbanerjee wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Added GHE enterprise connection option #25

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
079e0b0
Added GHE enterprise connection option
Mar 20, 2017
bc29e50
Update Readme.md
Mar 22, 2017
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
12 changes: 12 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,18 @@ Create `.auth.json` in the directory you will run `bin/sync` from:
}
```

### GitHub Enterprise

To connect to a GHE you will need to add the GHE host to the `.auth.json` file above:

```
{
"username": "github-username",
"password": "github-password",

@rmg rmg Mar 20, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be better to use token for the GHE example, IMNSHO, since basic auth support is optional and token auth is generally preferred in all circumstances.

"enterprise_host": "github.your-GHE-company.com"

@rmg rmg Mar 20, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since GHE exposes the same API and all of the clients treat it exactly the same, it would be better to just have a single GH URL option that simply defaults to https://api.github.com/.

@nikbanerjee nikbanerjee Mar 22, 2017

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So have a field for the host with api.github.com and allow it to be changed? Because the current way it works is it defaults to api.github.com if no enterprise_host is specified.

}
```

## bin/sync.js

Synchronize labels and milestones across multiple products.
Expand Down
17 changes: 14 additions & 3 deletions lib/create-client.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,27 @@ auth.password = process.env.GH_PASSWORD || auth.password;
console.assert(auth.username || auth.token);
console.assert(auth.password || auth.token);

var github = new GitHubApi({
var github_params = {
// required
version: "3.0.0",
// optional
protocol: "https",
//timeout: 5000,

cachedb: '_cache.db',
validateCache: false,
});
}

ghe_host = auth.enterprise_host

if (ghe_host) {
console.info("Connecting to GitHub Enterprise (" + ghe_host + ")..")
github_params.host = ghe_host;
github_params.pathPrefix = "/api/v3"

@rmg rmg Mar 20, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is part of the company specific URL and admins are actually free to use just about whatever root URL they like, so this is pretty specific to one company's GHE deployment.

@nikbanerjee nikbanerjee Mar 22, 2017

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would I have the user specify the pathPrefix as well then?

@rmg rmg Mar 22, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the module being used.. seems they made the rather unfortunate choice of splitting the URL like this. I thought it was smarter than that :-(

@rmg rmg Mar 22, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I would do is make the config use a url and then parse it on load. Something like this maybe?

var ghUrl = require('url').parse(auth.url || 'https://api.github.com');
github_params.host = ghUrl.host;
github_params.pathPrefix = ghUrl.path;
console.info('Connecting to: %s', ghUrl.format());

} else {
console.info("Connecting to GitHub..")
}

var github = new GitHubApi(github_params);

if ('token' in auth) {
github.authenticate({
Expand Down