-
Notifications
You must be signed in to change notification settings - Fork 13
adds a command for installing addons from npm #11
Conversation
function exists (addon, argv) { | ||
const deferred = Promise.defer() | ||
|
||
request.get(resolve(argv.registry, addon), function (err, res) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In cases of read-thru-cache for npmE, couldn't this return a 404 even though npm install
might work (by adding it to the whitelist)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking that we should default this URL to https://registry.npmjs.org
, even if someone happens to have a different registry configured for npm Enterprise. I probably shouldn't have even made this URL configurable.
This brings up a good point though, we should definitely make it so a proxy can be configured for these requests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've opened a separate issue for adding proxy support:
I went ahead and landed this so that I can test on replicated, I've created two follow up tickets that I will implement to address the code review. |
When you run
oauth2-server-pg install <addon>
it:I'm a terrible person and should write more tests for this; my plan is to loop around and add more tests once we have some end-to-end integration testing done.