Skip to content

fix: custom comfy-api-base works with subpath #8332

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

Merged
merged 1 commit into from
May 30, 2025

Conversation

BennyKok
Copy link
Contributor

Fix URL joining issue in API client causing incorrect endpoint paths

Problem

When using API endpoints with paths starting with /, the urljoin function was incorrectly treating them as absolute paths, causing the base URL path to be stripped out.

Example:

  • Base URL: https://api.comfydeploy.com/api/comfy-org/
  • Path: /proxy/stability/v2beta/stable-image/generate/ultra
  • Before: https://api.comfydeploy.com/proxy/stability/v2beta/stable-image/generate/ultra
  • After: https://api.comfydeploy.com/api/comfy-org/proxy/stability/v2beta/stable-image/generate/ultra

Root Cause

Python's urljoin() follows RFC 3986 standards where paths starting with / are treated as absolute paths, replacing the entire path portion of the base URL instead of appending to it.

Solution

Modified the ApiClient.request() method to strip leading slashes from paths before passing them to urljoin, ensuring all paths are treated as relative.

# Before
url = urljoin(self.base_url, path)

# After  
relative_path = path.lstrip('/')
url = urljoin(self.base_url, relative_path)

Impact

  • ✅ Fixes API endpoint URL construction for paths starting with /
  • ✅ Maintains backward compatibility with existing relative paths
  • ✅ Preserves urljoin functionality for proper URL handling
  • ✅ No breaking changes to existing API integrations

Files Changed

  • comfy_api_nodes/apis/client.py

@BennyKok BennyKok requested a review from comfyanonymous as a code owner May 29, 2025 17:22
@comfyanonymous comfyanonymous merged commit 6c319cb into comfyanonymous:master May 30, 2025
5 checks passed
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