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

Enhanced Download Functionality with Resume Support (Fix issue#59) #61

Closed
wants to merge 3 commits into from

Conversation

uxfion
Copy link

@uxfion uxfion commented Nov 14, 2024

Enhanced Download Functionality with Resume Support

This PR enhances the download functionality of proxpi with several major improvements focusing on reliability, user experience, and better handling of different file types.

Related issue #59

Key Improvements

  1. Smart Download Resume

    • Added support for resuming interrupted downloads for package files (wheels, tarballs)
    • Implements proper handling of Range headers and 206 Partial Content responses
    • Automatically detects server support for resume via accept-ranges header
  2. Differentiated Handling for Metadata Files

    • Special handling for .metadata files which cannot be resumed
    • Ensures data integrity by enforcing complete downloads for metadata files
    • Prevents corruption by cleaning up incomplete metadata downloads
  3. Enhanced Progress Reporting

    • Regular progress updates with download speed and ETA
    • Human-readable file sizes (bytes, KB, MB, GB, TB)
    • Colored output for better readability (package names in purple, errors in red)
  4. Improved Error Handling and Recovery

    • Implemented retry mechanism (configurable, default 3 attempts)
    • Proper cleanup of temporary files on failure
    • Detailed error messages with appropriate context

Technical Details

Download Resume Strategy

The download process follows this logic:

  1. For non-metadata files:

    if temporary file exists:
      check file size
      send HEAD request to get total size
      if server supports ranges and partial file is valid:
        resume from last byte
      else:
        start fresh download
    
  2. For metadata files:

    if temporary file exists:
      remove it (metadata must be downloaded completely)
    start fresh download
    

Progress Tracking

  • Updates every 30 seconds to avoid log spam
  • Shows:
    • Downloaded size / Total size
    • Percentage complete
    • Current speed (MB/s)
    • Estimated time remaining
  • Final summary includes average speed and total time

Example Output

Downloading numpy-1.21.0.whl... 
Progress: 5.25MB/10.50MB (50.0%) @ 2.1 MB/s - ETA: 2.5s

Successfully downloaded numpy-1.21.0.whl
Saved to: /cache/numpy-1.21.0.whl
Size: 10.50 MB @ 2.3 MB/s in 4.5s

Testing Notes

This has been tested with:

  • Various package sizes (from small metadata files to large wheels)
  • Different network conditions (stable/unstable connections)
  • Server responses with and without range support
  • Interrupted downloads with resume attempts

Migration Impact

This change is fully backward compatible and requires no configuration changes. It automatically enhances the download experience where supported and gracefully falls back to full downloads where needed.

@EpicWink
Copy link
Owner

EpicWink commented Nov 15, 2024

This pull request (PR) introduces too much functionality. Please split up this patch into multiple PRs. A good rule of thumb is if you have a bullet or numbered list in your PR summary, then each list item should be its own PR.


"Download resume" is very much not a piece of functionality that proxpi should have: it should either complete the download fully, or not leave a file in the cache at all. f3a77a0 exacerbates this, so I have implemented the atomic download your PR suggests in b0bdbc2.


Download progress logging is also not within the scope of proxpi: users shouldn't be looking at the logs at all, and they are really only there for investigating problems and debugging proxpi itself.


pip already performs retrying (see docs), why implement it as well in proxpi?

@uxfion uxfion closed this Nov 16, 2024
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

Successfully merging this pull request may close these issues.

2 participants