Skip to content

fix: use config.IMAGE_TIMEOUT for image URL download#151

Merged
CyberSecDef merged 2 commits into
mainfrom
copilot/fix-image-download-timeout
Apr 8, 2026
Merged

fix: use config.IMAGE_TIMEOUT for image URL download#151
CyberSecDef merged 2 commits into
mainfrom
copilot/fix-image-download-timeout

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 8, 2026

IMAGE_TIMEOUT was applied to the image generation POST but reverted to a hardcoded 60s for the subsequent URL download, silently ignoring user configuration.

Changes

  • novelforge/llm/image.py: Replace timeout=60 with timeout=config.IMAGE_TIMEOUT in the requests.get download call
# Before
img_resp = requests.get(image_url, timeout=60, stream=True)

# After
img_resp = requests.get(image_url, timeout=config.IMAGE_TIMEOUT, stream=True)
  • tests/test_image_download_timeout.py: New tests asserting the timeout kwarg passed to requests.get matches config.IMAGE_TIMEOUT for custom values (120s, 300s)

Copilot AI requested review from Copilot and removed request for Copilot April 8, 2026 20:07
Copilot AI linked an issue Apr 8, 2026 that may be closed by this pull request
Copilot AI requested review from Copilot and removed request for Copilot April 8, 2026 20:09
Copilot AI changed the title [WIP] Fix image download timeout hardcoded to 60s fix: use config.IMAGE_TIMEOUT for image URL download Apr 8, 2026
Copilot AI requested a review from CyberSecDef April 8, 2026 20:11
@CyberSecDef CyberSecDef marked this pull request as ready for review April 8, 2026 20:28
Copilot AI review requested due to automatic review settings April 8, 2026 20:28
@CyberSecDef CyberSecDef merged commit 3629e19 into main Apr 8, 2026
10 checks passed
@CyberSecDef CyberSecDef deleted the copilot/fix-image-download-timeout branch April 8, 2026 20:28
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes image URL download to respect the user-configured config.IMAGE_TIMEOUT (previously hardcoded to 60s), and adds regression tests to ensure the timeout value is propagated into the requests.get call.

Changes:

  • Use config.IMAGE_TIMEOUT for the image download (requests.get) timeout in call_image_api.
  • Add tests verifying requests.get(..., timeout=...) matches config.IMAGE_TIMEOUT for custom values (120, 300).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
novelforge/llm/image.py Applies config.IMAGE_TIMEOUT to the image URL download request.
tests/test_image_download_timeout.py Adds regression tests asserting download timeout uses config.IMAGE_TIMEOUT.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +3 to +4
import base64
import json
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

base64 and json are imported but not used in this test module. Removing unused imports will keep the tests clean and avoid failing future linting/static checks if introduced.

Suggested change
import base64
import json

Copilot uses AI. Check for mistakes.
Comment on lines +89 to +94
image_mod.call_image_api("sunset scene", filename_prefix="cover")

actual_timeout = get_calls[0]["kwargs"].get("timeout")
assert actual_timeout == 300, (
f"Expected timeout=config.IMAGE_TIMEOUT (300), got {actual_timeout!r}"
)
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_download_timeout_reflects_updated_config accesses get_calls[0] without first asserting that requests.get was called. Adding an explicit len(get_calls) == 1 assertion (as in the previous test) will produce a clearer failure if the download path changes.

Copilot uses AI. Check for mistakes.
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.

Image Download Timeout Hardcoded to 60s

3 participants