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

Allow alternative storage options like filesystem #9

Closed
bhanuc opened this issue Mar 4, 2016 · 20 comments
Closed

Allow alternative storage options like filesystem #9

bhanuc opened this issue Mar 4, 2016 · 20 comments

Comments

@bhanuc
Copy link
Contributor

bhanuc commented Mar 4, 2016

We have a specific use case of self hosting the registry on-demand on our server itself . I think it would be easy to create a pluggable system of switching storage backends. I can contribute with a pull request with a sample implementation.

@jdx
Copy link
Owner

jdx commented Mar 4, 2016

While this isn't an unreasonable request, it breaks one of the major goals of this project to make the app servers stateless. Feel free to fork this project if you would like to do that, but I wouldn't be able to accept a PR for it even as an optional configuration.

@raybooysen
Copy link

I came here from Sinopia and one of the reasons we wanted to use Sinopia was to ensure we didn't have to rely on the internet for builds. This would be a great feature to have.

@jdx
Copy link
Owner

jdx commented Mar 8, 2016

I made this a bit clearer in the README, but this is a 12 factor app http://12factor.net/ and as such adding filesystem storage as an option would make it stateful and break 12 factor.

My opinion is that you shouldn't have stateful servers ever. Too much can go wrong, they're difficult to backup, they crash, etc. S3 is a much better solution for package storage in all environments IMO.

But again, I do understand why you might want to do this, even though I think it's a bad idea. So feel free to fork if you'd like! It uses the ISC license so you don't have to get my permission or anything to do that.

@jdx jdx closed this as completed Mar 8, 2016
@raybooysen
Copy link

Sure. Unsure of what you mean by stateful. S3 is state, just happens to be somewhere else. Anyway, thanks for the feedback

@jdx
Copy link
Owner

jdx commented Mar 8, 2016

Stateful servers. S3 isn't a server, it's a service.

@raybooysen
Copy link

Like the file system as a service? :)

On 8 March 2016 at 15:40, Jeff Dickey [email protected] wrote:

Stateful servers. S3 isn't a server, it's a service.


Reply to this email directly or view it on GitHub
#9 (comment).

@jdx
Copy link
Owner

jdx commented Mar 8, 2016

I can elaborate. The problem with using the filesystem is now you have to manage the server. You need to back it up. You can't run a second server for failover or scaling since the files can only be on one box. With a stateless server it can be blown away and replaced at any time, which works well in cloud environments where servers can always disappear or fail.

Yes ultimately S3 is still a server technically, but it's not one that I manage, and that's the key difference. Stateful servers are much harder to manage and best to be avoided at all costs.

This does put a dependency on S3, but S3 is extremely reliable and it's not something I worry about going down.

@raybooysen
Copy link

Would you be interested in even a PR that pulled the S3 out into a provider model? I can fork your project but would prefer just to write my own provider and plug that in.

That would mean we don't have to fork and still get all the benefits as you say.

@jdx
Copy link
Owner

jdx commented Mar 8, 2016

sorry but no. I don't want the added complexity.

@raybooysen
Copy link

OK, that's unfortunate to hear. We're talking one function to return the s3
impl. Would have been nice to collaborate.

@adarshaj
Copy link

Is there a fork with the said fs support already? I'm interested.

@bhanuc
Copy link
Contributor Author

bhanuc commented Mar 15, 2016

I have made a fork here. I plan to pursue its development independent of this project. Feel Free to test/contribute to it. For now I have replaced fs with s3 but I plan to make it pluggable very soon. Thanks for this awesome project btw.

@jdx
Copy link
Owner

jdx commented Jul 28, 2016

It's been a few months and some people have been using this. I've come around a bit on this functionality, I think it does have some use-cases.

@bhanuc how is your implementation working for you? Do you think you could start a PR to add this in?

@jdx jdx reopened this Jul 28, 2016
@bhanuc
Copy link
Contributor Author

bhanuc commented Jul 28, 2016

@dickeyxxx I had moved the storage layer into a different module. I can create a similar module for s3 and fs based on the exisiting code, but that would require moving s3 specific code in a different module. It would also mean initialising the storage layer in config.js and using a storage agnostic api in the core module. I can start a PR, but let me know if you are ok with the changes mentioned above or suggest something else. I did something similar with my fork ,

@jdx
Copy link
Owner

jdx commented Jul 28, 2016

that sounds reasonable to me. I imagine the idea to detect which storage layer would be if AWS_S3_BUCKET is set or not?

@raybooysen
Copy link

Would be nice to.be configuration based. So not checking if an aws variable
exists but could be any plugin provider

On 28 Jul 2016 20:08, "Jeff Dickey" [email protected] wrote:

that sounds reasonable to me. I imagine the idea to detect which storage
layer would be if AWS_S3_BUCKET is set or not?


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#9 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAcMha87Z4L9J84iEnAEjnOcBr1SL4ekks5qaP5DgaJpZM4HpzS-
.

@jdx
Copy link
Owner

jdx commented Jul 28, 2016

Using a configuration file won't work on 12 factor services so it has to be configurable by environment variables.

@raybooysen
Copy link

Didn't mean a config file. Just meant that the absence of an AWS key
shouldn't define the provider.

On 28 Jul 2016 20:38, "Jeff Dickey" [email protected] wrote:

Using a configuration file won't work on 12 factor services so it has to
be configurable by environment variables.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#9 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAcMhSnvkS7z8ZdAq_f4EdpoEK28FZDLks5qaQU5gaJpZM4HpzS-
.

@jdx
Copy link
Owner

jdx commented Jul 28, 2016

Something like NPM_REGISTER_PROVIDER=s3|local might be ok

@bhanuc
Copy link
Contributor Author

bhanuc commented Jul 29, 2016

Kindly see changes in my latest fork here- https://github.com/bhanuc/npm-register . I cannot test the s3 specific module but it should mostly work. I took my own liberty at changing things, let me know if are ok with the changes.

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

No branches or pull requests

4 participants