Fail upload if it uses more then available space #220
Conversation
Before upload, check that more then the disk has more than the minimum free space. Upload up to the size limit that would exceed the available space. Fixes #179
gammazero
force-pushed
the
low-storage-fails-upload
branch
from
19a40f2
to
ad2be05
Compare
blob/local_store.go
Outdated
| } | ||
|
|
||
| // NewLocalStore instantiates a new LocalStore and uses the given dir as the place to store blobs. | ||
| // Blobs are stored as flat files, named by their ID with .bin extension. | ||
| func NewLocalStore(dir string) *LocalStore { | ||
| func NewLocalStore(dir string, minFreeSpace uint64) *LocalStore { |
There was a problem hiding this comment.
This new parameter might go better in a new config struct, since it's optional and what it represents is not obvious when reading a function call. Will also set up a better pattern for adding new options if ever needed in the future.
myStore := NewLocalStore("/a/dir", 1024) // what is 1024?
myStore := NewLocalStore("/a/dir", LocalStoreConfig { MinFreeSpace: 1024 }) // better in my opinion
There was a problem hiding this comment.
I would prefer to pass Option parameters instead of a config. If passing a config, we would still want a source of default values, which the option provides. I am implemented the optional parameter in the PR update.
As far as this: myStore := NewLocalStore("/a/dir", 1024)
... no one should ever do that anyway. Code like that should always use a const, never a raw number. For example:
const minFreeSpace = 2 * Gib // 2 Gib must remain free on disk
...
myStore := NewLocalStore("/a/dir", minFreeSpace)There was a problem hiding this comment.
nice, option pattern is even better
fair point about minFreeSpace as a param. i guess i can't back it up more than that it feels weird - i like to see fundamental, required fields as direct constructor params and optional peripheral fields to go in a config struct or option pattern.
definitely more philosophical than practical, would never deny a PR on these grounds, i just like discussing style : )
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #220 +/- ##
==========================================
+ Coverage 38.31% 40.17% +1.85%
==========================================
Files 6 7 +1
Lines 903 941 +38
==========================================
+ Hits 346 378 +32
- Misses 511 515 +4
- Partials 46 48 +2
|
Before upload, check that more then the disk has more than the minimum free space. Upload up to the size limit that would exceed the available space.
Fixes #179