Space check #70
Space check #70
Conversation
|
Whoops. Didn't see the work done and the comments to #64. |
|
@magnuhho Thanks for the series of PRs. I need to get travis working again properly for the repo (it changed orgs) and will give them a look-see. |
conda_mirror/conda_mirror.py
Outdated
| @@ -26,6 +26,7 @@ | |||
| 'win-64', | |||
| 'win-32'] | |||
|
|
|||
| MINIMUM_FREE_SPACE_MB = 1000 | |||
There was a problem hiding this comment.
It would help to make variable a command line argument instead so that users can decide what minimum means on their infrastructure and testing becomes more feasible.
There was a problem hiding this comment.
parente, apologies for the late reply.
A command line argument is a good idea, but "instead of" or "in addition to".
Don't you think a built-in default would be good so default behavior is to stop before the disks are full?
There was a problem hiding this comment.
A command line parameter with that default if not specified sounds good to me.
conda_mirror/conda_mirror.py
Outdated
| @@ -678,6 +679,10 @@ def main(upstream_channel, target_directory, temp_directory, platform, | |||
| with tempfile.TemporaryDirectory(dir=temp_directory) as download_dir: | |||
| logger.info('downloading to the tempdir %s', download_dir) | |||
| for package_name in sorted(to_mirror): | |||
| disk = os.statvfs(download_dir) | |||
There was a problem hiding this comment.
https://docs.python.org/3/library/os.html#os.fstatvfs suggests this function is Unix-only. Are you aware of a cross-platform solution for Windows users (which I think we have)?
There was a problem hiding this comment.
We can use shutil.disk_usage if we can assume Python 3.3 or newer?
https://docs.python.org/3/library/shutil.html#shutil.disk_usage
There was a problem hiding this comment.
We only test for 3.4+ and up (https://github.com/Valassis-Digital-Media/conda-mirror/blob/master/.travis.yml) and declare it's a Python 3 only package (https://github.com/Valassis-Digital-Media/conda-mirror#compatibility) so I think shutil.disk_usage sounds perfect.
|
added changes to
|
Codecov Report
@@ Coverage Diff @@
## master #70 +/- ##
========================================
- Coverage 93.89% 92.7% -1.2%
========================================
Files 2 2
Lines 262 274 +12
========================================
+ Hits 246 254 +8
- Misses 16 20 +4
Continue to review full report at Codecov.
|
|
Thanks for working through this @magnuhho. It looks reasonable now. We'll probably want to add a test for it in the future, but let's get this merged for now. 🎉 |
By default conda-mirror will continue pulling files until it either succeeds or dies from some sort of error.
This change aborts downloads with a logger.error when a threshold is reached.