Skip to content
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

Revert "Fix: File Descriptor leak" #6887

Merged
merged 1 commit into from
Feb 22, 2025
Merged

Revert "Fix: File Descriptor leak" #6887

merged 1 commit into from
Feb 22, 2025

Conversation

enyst
Copy link
Collaborator

@enyst enyst commented Feb 22, 2025

Reverts #6883

Sorry, I can't run main this morning, and the interesting part is that the errors are different for different providers. It must be working fine with Anthropic without proxy (direct API call), but not via litellm_proxy, and with log_completions disabled (I have it enabled).

With Claude, directly

 File "/Users/enyst/repos/odie/openhands/llm/llm.py", line 314, in wrapper
    f.write(json.dumps(_d))
...
  File "/Users/enyst/.pyenv/versions/3.12.6/lib/python3.12/json/encoder.py", line 180, in default
    raise TypeError(f'Object of type {o.__class__.__name__} '
TypeError: Object of type HTTPHandler is not JSON serializable

With Claude, via litellm proxy:

litellm.exceptions.APIError: litellm.APIError: APIError: Litellm_proxyException - 'HTTPHandler' object has no attribute 'api_key'

With o3-mini, directly:

litellm.exceptions.APIError: litellm.APIError: APIError: OpenAIException - 'HTTPHandler' object has no attribute 'api_key'

The first can be fixed by just popping out of kwargs the new client after the call, but the other two are non-obvious to me at first sight, WDYT?


To run this PR locally, use the following command:

docker run -it --rm   -p 3000:3000   -v /var/run/docker.sock:/var/run/docker.sock   --add-host host.docker.internal:host-gateway   -e SANDBOX_RUNTIME_CONTAINER_IMAGE=docker.all-hands.dev/all-hands-ai/runtime:bcf7b37-nikolaik   --name openhands-app-bcf7b37   docker.all-hands.dev/all-hands-ai/openhands:bcf7b37

@enyst enyst requested a review from tofarr February 22, 2025 10:47
Copy link
Collaborator

@tofarr tofarr left a comment

Choose a reason for hiding this comment

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

Ok. We can revisit the fix on Monday

@enyst enyst enabled auto-merge (squash) February 22, 2025 10:49
@enyst enyst merged commit bf82f75 into main Feb 22, 2025
14 checks passed
@enyst enyst deleted the revert-6883-fix-fd-leak branch February 22, 2025 11:21
tofarr added a commit that referenced this pull request Feb 23, 2025
@tofarr tofarr mentioned this pull request Feb 23, 2025
1 task
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