Token check#57
Conversation
…L validation and candidate handling
There was a problem hiding this comment.
Pull request overview
This PR improves the robustness of CivitAI model downloads by deferring URL validation from the candidate selection phase to the actual download phase, and enhances error handling with more informative messages that guide users on token usage for private models.
Changes:
- Modified HEAD request validation to log and continue on errors rather than fail, since some servers block HEAD requests
- Refactored candidate selection to collect all valid candidates upfront instead of selecting one after URL validation
- Enhanced error messages to provide guidance on using CivitAI tokens for authentication
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| except requests.HTTPError as e: | ||
| # Log but continue - HEAD requests may fail even when GET succeeds | ||
| status_code = e.response.status_code if hasattr(e, 'response') else 'unknown' | ||
| logger.info(f"HEAD request returned {status_code} for {url}, attempting download anyway...") |
There was a problem hiding this comment.
The exception handling for HEAD validation only catches requests.HTTPError, but the validate_url_with_head function can also raise other requests.exceptions.RequestException types (e.g., connection errors, timeouts). These exceptions should also be caught and logged to maintain consistent behavior with the underlying validation function.
| except requests.HTTPError as e: | |
| # Log but continue - HEAD requests may fail even when GET succeeds | |
| status_code = e.response.status_code if hasattr(e, 'response') else 'unknown' | |
| logger.info(f"HEAD request returned {status_code} for {url}, attempting download anyway...") | |
| except requests.exceptions.RequestException as e: | |
| # Log but continue - HEAD requests may fail even when GET succeeds | |
| status_code = getattr(getattr(e, "response", None), "status_code", "unknown") | |
| logger.info( | |
| f"HEAD request encountered {e.__class__.__name__} " | |
| f"(status: {status_code}) for {url}, attempting download anyway..." | |
| ) |
| # Exception handling when search candidates are not found | ||
| if not selected_model: | ||
| # Add to candidates list (skip URL validation which causes 403 errors) | ||
| # Actual download will be attempted and retried recursively |
There was a problem hiding this comment.
The comment mentions download will be "retried recursively" but the implementation uses an iterative loop (for loop over candidates), not recursion. Consider updating the comment to say "retried iteratively" or "retried for each candidate" for accuracy.
| # Actual download will be attempted and retried recursively | |
| # Actual download will be attempted and retried iteratively for each candidate |
| # Handle file download and setting model information | ||
| if download: | ||
| # Try downloading from the models list, retrying on 401/403 errors | ||
| # Try downloading from all candidates recursively |
There was a problem hiding this comment.
The comment says "Try downloading from all candidates recursively" but the implementation is iterative (a for loop), not recursive. Consider updating to "Try downloading from all candidates sequentially" or "Try downloading from all candidates in order" for clarity.
| # Try downloading from all candidates recursively | |
| # Try downloading from all candidates sequentially (in order) until one succeeds |
| file_name = candidate_model["filename"] | ||
| download_url = candidate_model["download_url"] |
There was a problem hiding this comment.
The assignments of file_name and download_url from candidate_model are redundant here since they were already assigned from the same source on lines 1526-1527 earlier in the same try block, and their values haven't changed. These lines can be removed to reduce code duplication.
| file_name = candidate_model["filename"] | |
| download_url = candidate_model["download_url"] |
| # Update selected metadata on success | ||
| download_success = True | ||
| logger.info(f"Successfully downloaded model from: {download_url}") | ||
| selected_model = candidate_model |
There was a problem hiding this comment.
Variable selected_model is not used.
| selected_model = candidate_model |
This pull request improves the robustness and user experience of the CivitAI model search and download pipeline by handling download URL validation more gracefully and providing better error handling and messaging. The main changes focus on collecting all candidate models, deferring URL validation to the actual download step, and improving the retry mechanism and user feedback when downloads fail.
Improvements to download and candidate selection logic:
file_downloaderwas updated to log and continue on errors, since some servers (like Cloudflare R2) block HEAD requests even when GET would succeed.Error handling and user feedback enhancements:
Code simplification and maintainability:
These changes make the pipeline more robust to server-side restrictions and improve the user experience when searching for and downloading models from CivitAI.