Skip to content

Comments

[FIX] add more time before timeout when lora are used (5s -> 5min)#12

Open
TristanValentin07 wants to merge 1 commit intomainfrom
fix/patch-flux-lora-not-working
Open

[FIX] add more time before timeout when lora are used (5s -> 5min)#12
TristanValentin07 wants to merge 1 commit intomainfrom
fix/patch-flux-lora-not-working

Conversation

@TristanValentin07
Copy link
Collaborator

@TristanValentin07 TristanValentin07 commented Oct 16, 2025

Edit the timeout of lora loading, to be able to load an image with a lora in flux, before reach the timeout of 5 seconds.

Summary by CodeRabbit

  • Bug Fixes
    • Added request timeout protection to prevent indefinite hangs during network operations. Requests will now timeout after 5 minutes of inactivity.

@coderabbitai
Copy link

coderabbitai bot commented Oct 16, 2025

Walkthrough

The HTTP client in queue_prompt now enforces a 300-second timeout via httpx.Timeout(300.0) wrapper. Previously, the client had no explicit timeout constraint. Request execution and exception handling logic remain unchanged.

Changes

Cohort / File(s) Summary
HTTP client timeout constraint
simpleai_base/comfyclient_pipeline.py
Added 300-second timeout wrapper to HTTP client in queue_prompt function to prevent indefinite request hangs

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

A rabbit hops through code so bright,
Adding timeouts left and right,
No endless waits in the network queue—
Three hundred seconds, then we're through! 🐰⏱️
Safety wrapped in one small line,
Now every request will be just fine!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly summarizes the core change by specifying that the timeout is increased from 5 seconds to 5 minutes when using LoRA, reflecting both the context and the key numeric adjustment made in the code.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/patch-flux-lora-not-working

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 2b242e7 and 6942ac8.

📒 Files selected for processing (1)
  • simpleai_base/comfyclient_pipeline.py (1 hunks)
🔇 Additional comments (1)
simpleai_base/comfyclient_pipeline.py (1)

28-28: LGTM with notes on PR description accuracy and timeout consistency.

The 300-second timeout is appropriate for LoRA loading operations and the implementation is correct. However, there's a minor inconsistency: the PR description mentions changing from "5s -> 5min", but the code shows no previous explicit timeout was set.

Additionally, other HTTP operations in this file lack timeouts and could potentially hang indefinitely:

  • get_image (line 41)
  • get_history (line 46)
  • interrupt (line 165)
  • free (line 176)
  • upload_mask (line 21)

Consider adding appropriate timeouts to other HTTP operations for consistency and robustness. For example:

# In get_image (line 41)
with httpx.Client(timeout=httpx.Timeout(60.0)) as client:

# In get_history (line 46)
with httpx.Client(timeout=httpx.Timeout(30.0)) as client:

# In interrupt and free (lines 165, 176)
with httpx.Client(timeout=httpx.Timeout(10.0)) as client:

# In upload_mask (line 21)
response = httpx.post("http://{}/upload/mask".format(server_address), 
                      files=files, data=data, timeout=60.0)

Run this script to verify if there are any other HTTP operations without timeouts:

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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