Skip to content
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.

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

best practices for managing clouds #62

Open
rlidwka opened this issue Sep 6, 2014 · 25 comments
Open

best practices for managing clouds #62

rlidwka opened this issue Sep 6, 2014 · 25 comments
Labels

Comments

@rlidwka
Copy link
Contributor

rlidwka commented Sep 6, 2014

I think I just found a nice name for this lot of orgs. "CLOUD". That's short for "Crap-Load Of Useful Dependencies".

Anyway, one npm dependency costs a lot. I mean, it's two http requests made by npm for each install of the package, means for every user. Try "npm install browserify" - it takes half a minute to install because of those countless plugins.

So I wonder how to reduce that.

Maybe suggest people to bundle all packages less than N kilobytes in size? Does it make sense to do so for express.js for example?

This question doesn't directly relate to jshttp, but I feel if we don't sort this out, some people (like hapi with vary) are going to reinvent their own buggy wheel instead.

@rlidwka rlidwka added the discuss label Sep 6, 2014
@jonathanong
Copy link
Member

This is really just an npm issue. Tell them to stop sucking.

But we could also bundle pinned dependencies. I don't see why not.

@dougwilson
Copy link
Contributor

I was looking at bundled dependencies not too long ago and needed some answers before doing that:

Does it simply include the node_modules dir in the tar ball? Is there one tar ball for each bundled dependency? If I accidentally have a changed file inside the node_modules folder in a dependency that is going to be bundled (usually debugging something), will that change end up getting bundled?

@rlidwka
Copy link
Contributor Author

rlidwka commented Sep 6, 2014

This is really just an npm issue. Tell them to stop sucking.

Those issues were closed with the label isaacs-says-no. :D

@Raynos
Copy link
Member

Raynos commented Sep 6, 2014

@rlidwka this is a non existant problem.

Locally you should use a npm-shrinkwrap.json file where npm install takes zero time to install if there are no differences.

Sure you have to download dependencies for every project you clone BUT you also had to clone it. Do you want to also delete all git history so that cloning a project is cheap ?

If you really care about download speed run a local caching proxy and point your registry at npm config set registry http://localhost:9999

If you care about the amount of time that it takes to install dependencies when running in CI or deployment then your doing it run, point CI & deployment at a caching proxy. Our node.js deploys at uber have a really fast npm install experience because of the caching we have set up on our production servers.

@rlidwka
Copy link
Contributor Author

rlidwka commented Sep 6, 2014

Does it simply include the node_modules dir in the tar ball? Is there one tar ball for each bundled dependency?

No, it's just one tarball for your package. You can download npm tarball itself from the registry and see how it's bundled. They use bundled deps everywhere.

If I accidentally have a changed file inside the node_modules folder in a dependency that is going to be bundled (usually debugging something), will that change end up getting bundled?

Yeah, it will. npm avoids this issue, because they check out modules to git, so you can check this easily. Oh well, that seems to be a disadvantage here.

@rlidwka
Copy link
Contributor Author

rlidwka commented Sep 6, 2014

@Raynos , we're talking about install time for our users, not for ourselves.

@dougwilson
Copy link
Contributor

@Raynos, we don't even bother running npm install for deployments and instead our deployments are static zipballs with all their dependencies already included. This is so we do not have to care if npm is down and we can deploy that zipball 2 months later and it'll work (shrinkwrap does not solve the issue that authors can unpublish a package from npm; it's sad npm has this problem).

@Raynos
Copy link
Member

Raynos commented Sep 6, 2014

@dougwilson we have a similar thing, except we have a mirror of npm that doesnt delete anything and users dont build their dependencies themself, we have a build server that builds tarballs from git shas.

@Raynos
Copy link
Member

Raynos commented Sep 6, 2014

@rlidwka we should educate users on how to use npm well, not work around the problem.

@rlidwka
Copy link
Contributor Author

rlidwka commented Sep 6, 2014

@Raynos , do you really expect people who just installed node.js and have no idea what npm is to go and set up private npm mirror?

@dougwilson
Copy link
Contributor

Anyway, if it picks up changes in node_modules dir of the author, it raises the chances of a bad package being published, especially when node_modules is not VC-tracked so it's not obvious if there are changes.

Probably adding "rm -rf node_modules" to prepublish may help, except then I couldn't publish any modules since windows does not have rm binary, lol.

@dougwilson
Copy link
Contributor

I would say that if any dependencies are going to be bundled, they should also get checked into git as a good rule.

@Raynos
Copy link
Member

Raynos commented Sep 6, 2014

@rlidwka no a private mirror is only for deploying at scale.

If your a small startup use nodejitsu, heroku, concurix or any other Paas aimed at node and they will do all this for you.

If you care about making installs faster beyond writing your app and using npm-shrinkwrap.json set up a localhost cached proxy, but that's an advanced use case.

A coworker is working on an open source local npm cache ( https://github.com/lxe/supermoon ).

Install time only matters when you start a new project, clone a project or update a dependency, it's no big deal.

@rlidwka
Copy link
Contributor Author

rlidwka commented Sep 6, 2014

I would say that if any dependencies are going to be bundled, they should also get checked into git as a good rule.

Into master? Or keep separate branch just for releasing stuff?

@Raynos
Copy link
Member

Raynos commented Sep 6, 2014

Please do not bundle or check your dependencies into git for libraries.

This is a terrible idea.

  • bundleDependencies break with mirrors because of a outstanding npm shrinkwrap bug.
  • I need to be able to change your dependencies in case there is a bug in the tree, I want npm update X to work.
  • when I want to contribute to your library it's really weird that node_modules is checked into git, should I commit any changes to node_modules when I make a PR, what's the process around this?

@rlidwka
Copy link
Contributor Author

rlidwka commented Sep 6, 2014

when I want to contribute to your library ..., should I commit any changes to node_modules when I make a PR

PR against dependency + ask to update. It's exactly the same thing as with pinned deps here.

@Raynos
Copy link
Member

Raynos commented Sep 6, 2014

If your going to check dependencies into git please only do this for a special snowflake top level kitchensink module.

Please do not do this for individual modules, that would be weird. using npm differently from the rest of the community is strange.

@dougwilson
Copy link
Contributor

FWIW I am also not very inclined to use bundled dependencies, mainly because there are various issues surrounding it, though other projects are free to do it if they wish. Of course other ways to reduce user install time is globally installing common modules like debug.

Yes, npm makes two requests, but classic cpan clients download the entire index up front and then have to decompress and read it, which sometimes takes a minute just doing that, so at least npm doesn't have that issue.

Also, doesn't npm keep a local cache of the tarballs anyway (only applies to reinstalls)?

@rlidwka
Copy link
Contributor Author

rlidwka commented Sep 6, 2014

If your going to check dependencies into git please only do this for a special snowflake top level kitchensink module.

It isn't really about us using that, but rather about us telling people how to use smallish jshttp modules in other frameworks.

Of course other ways to reduce user install time is globally installing common modules like debug.

Won't work. Global modules can't be require'd because they are outside of NODE_PATH.

@Raynos
Copy link
Member

Raynos commented Sep 6, 2014

@rlidwka telling people to check node_modules into git is fine. Just link them to mikeals blog post.

Checking the dependencies of each module under the jshttp repo into git is what I was complaining about.

@dougwilson
Copy link
Contributor

Checking the dependencies of each module under the jshttp repo into git is what I was complaining about.

Ah. That is not going to happen in these repos. Yea, @rlidwka is asking about guidelines for people who consume jshttp.

@Raynos
Copy link
Member

Raynos commented Sep 7, 2014

@rlidwka sorry about the confusion!

You are right, we can encourage people to check those into git :)

@Fishrock123
Copy link
Member

Ok so, multi-stage-installs are making progress in npm, that's a plus I think.

Looks like using bundledDependancies will one day be viable, maybe? Not sure if it's really a solution though haha.

http://blog.npmjs.org/post/98233700815/multi-stage-installs-and-a-better-npm

@Fishrock123
Copy link
Member

cc @Raynos

bundleDependencies break with mirrors because of a outstanding npm shrinkwrap bug.

I couldn't find an issue on npm for this, could you point to it, or is it resolved?

@Raynos
Copy link
Member

Raynos commented Oct 23, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants