-
Notifications
You must be signed in to change notification settings - Fork 38
feat(llm): add enterprise gateway and SSL verification support #1026
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?
feat(llm): add enterprise gateway and SSL verification support #1026
Conversation
Coverage Report •
|
||||||||||||||||||||||||||||||
tests/sdk/llm/test_llm.py
Outdated
| """Test that ssl_verify is correctly parsed from environment variables.""" | ||
| # Test various false values | ||
| for value in ["false", "False", "FALSE", "0", "no", "off"]: | ||
| os.environ["LLM_SSL_VERIFY"] = value |
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.
@OpenHands there's some monkeypatch env in pytest, which cleans up itself, for use in testing. You know what I mean. Adjust this test a bit to use it
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.
I'm on it! enyst can track my progress at all-hands.dev
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.
Summary of changes
What I updated
- File: tests/sdk/llm/test_llm.py
- Test updated: test_llm_ssl_verify_env_parsing
- Switched from direct os.environ manipulation to pytest’s monkeypatch.setenv for environment variables.
- Set LLM_MODEL once via monkeypatch for all load_from_env calls in the test.
- Removed manual cleanup of environment variables and the now-unused os import.
Why
- Per the PR comment, this uses pytest’s monkeypatch fixture which automatically restores environment state after the test, improving isolation and avoiding side effects.
Verification
- Pre-commit on the changed file: passed (ruff format/lint, pycodestyle, pyright).
- Ran the specific test: uv run pytest -q tests/sdk/llm/test_llm.py::test_llm_ssl_verify_env_parsing → 1 passed.
Checklist
- Addressed the request to use pytest’s env monkeypatch.
- Kept the change minimal: only modified the relevant test and removed an unused import.
- Followed repo dev/test guidelines (pre-commit hooks, no mypy, minimal changes).
- Committed only the changed test file with Co-authored-by and pushed to the current branch.
Commit
- tests(llm): use pytest monkeypatch for env vars in ssl_verify parsing test
- Branch: alona/all-4119-add-enterprise-gateway-support-for-llm-providers-in-cli-v3
If you want, I can sweep other tests for os.environ usage and convert them to monkeypatch as well.
… test Replaces direct os.environ manipulation with monkeypatch.setenv so env is auto-restored and the test is isolated. Co-authored-by: openhands <[email protected]>
- Wrap long strings to satisfy line length - Replace cli_mode usage not supported by Agent ctor - Remove trailing whitespace per pycodestyle Co-authored-by: openhands <[email protected]>
…t-for-llm-providers-in-cli-v3
|
Looks like there are a few issues preventing this PR from being merged!
If you'd like me to help, just leave a comment, like Feel free to include any additional details that might help me get this PR into a better state. You can manage your notification settings |
enyst
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.
Just to note, the docs check is not mandatory. It's there just to remind us to make a docs PR (the agent should know what to do if we tell it to do something similar to the other examples)
Agent Server images for this PR
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.12-nodejs22golang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:6a8a73a-pythonRun
All tags pushed for this build
About Multi-Architecture Support
6a8a73a-python) is a multi-arch manifest supporting both amd64 and arm646a8a73a-python-amd64) are also available if needed