-
Notifications
You must be signed in to change notification settings - Fork 28
Add Fal.ai generation service wrapper #312
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| "openrouter": OpenRouterService, | ||
| "local": LocalService, | ||
| "stabilityai": StabilityAIService | ||
| "local": LocalService, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Duplicate SERVICE_MAP entries introduced by merge error
The diff introduces duplicate entries that appear to be merge artifacts. The "local": LocalService key appears twice in SERVICE_MAP (lines 17-18), and StabilityAIService is imported twice (lines 8-9). While Python dictionaries silently overwrite duplicate keys, this indicates a merge error and the duplicate "local" entry may have been intended to be a different service entry that was lost during the merge.
Additional Locations (1)
| poll_interval = 2.0 | ||
|
|
||
| while time.time() - start_time < timeout: | ||
| status_url = f"{self.base_url}/requests/{request_id}/status" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Status URL missing model path breaks polling
The status URL and result URL are constructed incorrectly. Jobs are submitted to {base_url}/{model} (e.g., https://queue.fal.run/fal-ai/flux/dev), but the status and result URLs use {base_url}/requests/{request_id} instead of {base_url}/{model}/requests/{request_id}. The model path is missing from the status and result polling endpoints, which will cause HTTP 404 errors when checking job status.
Additional Locations (1)
|
|
||
| else: | ||
| bt.logging.warning(f"Unknown Fal.ai status: {status}") | ||
| time.sleep(poll_interval) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Timeout ineffective when HTTP requests hang indefinitely
The 600-second timeout at line 124 is ineffective because the requests.get() and requests.post() calls (lines 107, 129, 138, 179) have no individual request timeouts. If any HTTP request hangs due to network issues, the code blocks indefinitely since the while loop condition at line 127 is only checked between iterations. The intended 10-minute timeout never triggers if a single request never returns.
dylanuys
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please confirm whether this provides valid c2pa certs or not
|
Yes it provides |
| if modality == "image": | ||
| # FLUX returns 'images': [{'url': ..., ...}] | ||
| if "images" in data and len(data["images"]) > 0: | ||
| media_url = data["images"][0]["url"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Image URL extraction lacks defensive key check
The image URL extraction at line 169 directly accesses data["images"][0]["url"] without verifying that the "url" key exists in the image object. In contrast, the video handling at lines 172-173 properly checks "url" in data["video"] before accessing it. If the API returns an image object without a "url" key, this code raises a confusing KeyError instead of falling through to the intended RuntimeError with a helpful message about "No media URL found".
|
@dylanuys please help look at this pull request, it passed all the checks |
This PR introduces support for Fal.ai as a new generation service provider. With this integration, miners and validators can now use Fal.ai for both image and video generation tasks, expanding the available options beyond OpenAI, OpenRouter and Stability AI.
Key Changes
FalAIServiceinneurons/generator/services/fal_service.py.fal-ai/flux/devmodel.fal-ai/kling-video/v1/standard/text-to-videomodel.neurons/generator/services/service_registry.pyto include"fal"inSERVICE_MAP.tests/generator/fal_service.pywith both positive and negative test cases.Usage
To use the Fal.ai service:
Note
Adds Fal.ai-based image and video generation with queue polling, integrates it into the service registry, and introduces a focused test suite.
neurons/generator/services/fal_service.py):image(modelfal-ai/flux/dev) andvideo(modelfal-ai/kling-video/v1/standard/text-to-video).FAL_KEY."fal"toSERVICE_MAPand includes Fal in API key requirement descriptions inget_all_api_key_requirements.tests/generator/fal_service.py: end-to-end generation tests for image/video, C2PA check for images, and invalid API key negative test.Written by Cursor Bugbot for commit 04d8f20. This will update automatically on new commits. Configure here.