Warn if parsed memory/scratchsize may be too small #510
Conversation
obbardc
force-pushed
the
wip/obbardc/debos-warn-low-size
branch
from
4ba7ea3 to
13f6822
Compare
|
Something is wrong with --scratchsize: While is works for --memory, the conversion for scratchsize doesn't give us the same value 100MB vs 95MB |
I guess that is because for scratchsize we convert to bytes with Perhaps we should unify this by:
To be honest, I threw this together quickly just for some feedback. Happy to rework it. |
|
I would say let's use for both |
obbardc
changed the title
go-units isn't great For this PR assuming a MB for disk is |
obbardc
force-pushed
the
wip/obbardc/debos-warn-low-size
branch
from
13f6822 to
88132f8
Compare
|
@sjoerdsimons made teh changes according to your suggestions :-) |
There was a problem hiding this comment.
Sorry i should have looked at the sizes more closely yesterday, i just ended up being in the 1024 vs 1000 rathole too much ;) TL;DR I think giving warnings is great, but the current threshold seems a bit low, if you run on a small VM (e.g. 1 or 2G memory instance) then having a smaller then 2G memory size is fine. Especially if you use a scratchdisk, where the VMs memory mostly just needs to be enough to run apt.
So lets be a bit less trigger-happy on these warnings, as you said it's mostly for people forgetting to add M. Which we'll still easily filter out even with a very conservative "accepted" minimum
Since the --memory and --scratchsize arguments are parsed with human-readable suffixes into bytes (e.g 2048MB or 2GB), if a user does not put a suffix the arguments are parsed as bytes which can cause issues later when a user sets the memory to be 2048 assuming it will be parsed as MB. Change the documentation to match reality and also add a warning if the user has set either parameter less than a recommended minimum. Closes: #509 Signed-off-by: Christopher Obbard <[email protected]>
obbardc
force-pushed
the
wip/obbardc/debos-warn-low-size
branch
from
88132f8 to
fe5f7e9
Compare
Since the --memory and --scratchsize arguments are parsed with human-readable suffixes into bytes (e.g 2048MB or 2GB), if a user does not put a suffix the arguments are parsed as bytes which can cause issues later when a user sets the memory to be 2048 assuming it will be parsed as MB.
Change the documentation to match reality and also add a warning if the user has set either parameter less than the recommended default.
Closes: #509