Skip to content

Conversation

Copy link

Copilot AI commented Sep 9, 2025

This PR consolidates the docker blob download functionality into the existing common_download_file_single function, eliminating code duplication and simplifying the download architecture.

Changes Made

1. Enhanced common_download_file_single function

  • Added a new bool is_docker parameter (defaults to false)
  • When is_docker=true, the function uses simplified download logic suitable for Docker registry blobs:
    • Skips complex caching/metadata handling (no .json files, no ETag/Last-Modified checks)
    • Performs direct file existence check for caching
    • Uses .tmp temporary files with atomic rename
    • Maintains all necessary CURL options and error handling

2. Removed common_docker_download_blob function

  • Eliminated the standalone docker download function entirely
  • The logic has been consolidated into common_download_file_single when is_docker=true
  • Removed both CURL and non-CURL versions of the function

3. Updated docker model resolution flow

  • Modified common_docker_resolve_model to use common_download_file_single(url, path, token, false, true)
  • Moved docker URL resolution from common_init_from_params to common_params_handle_model
  • This ensures docker URLs are resolved earlier in the parameter handling pipeline

4. Updated existing function calls

  • All existing calls to common_download_file_single now explicitly pass is_docker=false
  • Maintains backward compatibility for regular HTTP downloads with full caching/metadata support

Benefits

  • Code consolidation: Eliminates ~70 lines of duplicate download logic
  • Maintainability: Single function to maintain for all download scenarios
  • Consistency: Both regular and docker downloads use the same error handling and CURL configuration
  • Architecture: Better separation of concerns with docker resolution happening in parameter handling

Backward Compatibility

This change is fully backward compatible:

  • Docker downloads maintain identical behavior (same CURL options, headers, error handling)
  • Regular HTTP downloads are unchanged
  • All existing APIs and function signatures preserved (with optional parameter addition)

The refactoring successfully addresses the original issue by removing common_docker_download_blob and using common_download_file_single with an is_docker parameter, while moving docker resolution to the appropriate location in the parameter handling flow.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • ggml.ai
    • Triggering command: /home/REDACTED/work/llama.cpp/llama.cpp/build/bin/test-arg-parser (dns block)
  • huggingface.co
    • Triggering command: /usr/lib/git-core/git-remote-https origin REDACTED (dns block)
    • Triggering command: /home/REDACTED/work/llama.cpp/llama.cpp/build/bin/test-thread-safety -hf ggml-org/models -hff tinyllamas/stories15M-q4_0.gguf -ngl 99 -p The meaning of life is -n 128 -c 256 -ub 32 -np 4 -t 2 (dns block)
    • Triggering command: /home/REDACTED/work/llama.cpp/llama.cpp/build/bin/llama-eval-callback --hf-repo ggml-org/models --hf-file tinyllamas/stories260K.gguf --model stories260K.gguf --prompt hello --seed 42 -ngl 0 (dns block)

If you need me to access, download, or install something from one of these locations, you can either:


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

…le with is_docker parameter

Co-authored-by: ericcurtin <[email protected]>
Copilot AI changed the title [WIP] Instead of implementing docker blob pulling in common_docker_download_blob, remove that function and use common_download_file_single instead. In common_download_file_single add a "bool is_docker" parameter for any differences required for docker blog ... Refactor docker download logic to use common_download_file_single with is_docker parameter Sep 9, 2025
Copilot AI requested a review from ericcurtin September 9, 2025 12:11
Copilot finished work on behalf of ericcurtin September 9, 2025 12:11
@ericcurtin ericcurtin force-pushed the docker-pull-functionality branch 3 times, most recently from 504f460 to d166c42 Compare September 9, 2025 21:21
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