facebookarchive / rocks-strata

Other
133 stars 24 forks source link

Add Minio as a storage option #16

Closed kulshekhar closed 8 years ago

kulshekhar commented 8 years ago

Resolves #15

igorcanadi commented 8 years ago

Thanks for your contribution!

BTW I find the directory structure a bit weird, since now we have a minio file under 'strata/cmd/mongo/lreplica_s3storage_driver' directory. Can you create a separate directory 'strata/cmd/mongo/lreplica_miniostorage_driver'? It's fine to have two binaries, at least for now.

Alternative would be to move everything into a 'strata/cmd/mongo' directory.

What do you think?

kulshekhar commented 8 years ago

Yes, the directory structure does look non-intuitive. One reasonable way it could be refactored is by renaming the lreplica_s3storage_driver to lreplica_drivers. This would also help avoid creating multiple binaries.

This would, however necessitate changes in the following files:

If this refactor is acceptable, I can make these changes.

I also just noticed that I'll have to make changes in the main.go file (in mongoq) on the same lines as the changes made in the main.go file (in strata).

igorcanadi commented 8 years ago

Yes, the directory structure does look non-intuitive. One reasonable way it could be refactored is by renaming the lreplica_s3storage_driver to lreplica_drivers. This would also help avoid creating multiple binaries.

Yeah this sounds good. Maybe do this in a separate PR first and then come back to this one?

I also just noticed that I'll have to make changes in the main.go file (in mongoq) on the same lines as the changes made in the main.go file (in strata).

Refactor it out into a storage_factory.go?

kulshekhar commented 8 years ago

Yeah this sounds good. Maybe do this in a separate PR first and then come back to this one?

Sounds good. I'll get on it.

Refactor it out into a storage_factory.go?

I'm not quite sure how this will be done. I'll have to take a closer look at this.

kulshekhar commented 8 years ago

I've refactored the directory name in PR #17. Once that's merged, I'll make the next set of changes.

Edit: Closing this for now.