npm / npm-registry-client

http://npm.im/npm-registry-client
ISC License
264 stars 108 forks source link

add support for scope packages in private npm registries #101

Closed teddyhwang closed 9 years ago

teddyhwang commented 9 years ago

The URI for the tarball when publishing a module seems to be broken for modules with scopes. Scopes are (currently) only supported in private registries with users who have admin access to the registry DB. The module name is not correctly escaped when the URI is generated.

totherik commented 9 years ago

Yeah, this is interesting. I wouldn't necessarily say this is an artifact of private registries, specifically, but instead of any registry that uses the Couch app to store attachments. I suspect that since in the main npm registry serves tarballs from a filesystem-like storage mechanism (CDN backed by an origin server), the un-escaped paths more realistically reflect the packages/resource's location on "disk", while in Couch the rewrites obviously require the discreet (escaped) name.

Either way, I'm running into this issue as well and I'd be curious to know the vision for how this is intended to work (e.g. will storing tarballs as attachments in couch always be supported?) just so I know whether or not to wait on this or start thinking about alternative solutions.

othiym23 commented 9 years ago

Both npm Enterprise and what npm engineering is calling registry-2 (which is basically npm Enterprise, but for the primary npm registry) have services that handle tarball publishes coming from npm-registry-client. Also, both sinopia and cnpmjs have been updated to support the publishing of scoped packages (many months ago, in both cases). So, scoped packages are getting published to "private registries" today.

I think in all of those cases, the registries either aren't using CouchDB (sinopia), or are using something like npm's skimdb where the packages aren't stored directly in CouchDB. So I think what you're asking for (correct me if I'm wrong) is support for the old fullfatdb-style registries. My understanding is that those are deprecated, for the simple reason that having attachments in the CouchDB starts to become an operational nightmare past a certain size. This is why we discontinued our fullfat replica sometime last year – it was unsupportable and, by the end, almost nobody was using it anyway.

This is a dangerous change to accept as-is, because I don't know what it would do to npm's existing infrastructure. I'd need @bcoe and / or @ceejbot to take a look at this and weigh in on what the implications would be for the new registry code, which they're itching to get into production (i.e. there are some inelastic constraints this is up against, in terms of acceptable risk). My guess is that even were we to be able to accomodate this, it would be a backwards compatibility-breaking change.

@totherik The specific guidance I can give you is that I don't think there's a single person at npm who thinks storing packages as attachments directly in CouchDB is a good idea.

totherik commented 9 years ago

Point well taken. I know the history of fullfatdb and didn't expect it would be something anyone at npm advocates. Was just surprised that the behavior was implicitly deprecated from npm-registry-couchapp without being removed.

For npm-registry-couchapp based registries that don't expect to reach a size that would cause said operational nightmare (e.g. a scoped registry for just my team to use), it seems using attachments should be just fine and not broken out of the box.

othiym23 commented 9 years ago

Speaking only from my own limited perspective on the CLI team, I think npm-registry-couchapp is entering a twilight period. npm still relies upon CouchDB for its replication support (which is great) and keeps the core registry metadata in there, but more and more of what it used to do is being broken out into independent services (cf. ref. ceej's talk at Node Summit). Features haven't been removed from npm-registry-couchapp because there's no reason to remove them, but the new registry architecture doesn't use them. fullfatdb, in particular, should be considered a deprecated architectural style for deploying npm-registry-couchapp.

totherik commented 9 years ago

Sounds good, thanks Forrest.

mmalecki commented 9 years ago

This appears to break the way we store tarballs, resulting in tarball URLs like

http://127.0.0.1:8080/@mmalecki/npm/-/@mmalecki%2fnpm-2.7.4.tgz vs
http://127.0.0.1:8080/@mmalecki/test-0/-/test-0-2.0.0.tgz

(Full name in the basename of the file instead of just the project name.)

cc @bcoe @ceejbot

othiym23 commented 9 years ago

This is incompatible with the way the primary registry (and npm On-Site) store tarballs, and because running an npm-registry-couchapp instance as a fullfat is more or less guaranteed to break in the very near term, it doesn't make sense to land this at all. Take a look at using sinopia or registry-static if you want a private registry. Thanks for putting this together!