Add CLI options for backend args (like headers and verify) #230
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sjmonson
reviewed
Jul 15, 2025
|
Ok thanks for feedback, will look into those changes |
kdelee
changed the title
Adds command-line options to the "guidellm benchmark" command to support custom authentication headers and skipping SSL verification when communicating with the target system. New options: --target-header: Allows specifying a custom header(s) for the target. --target-skip-ssl-verify: A flag to disable SSL certificate verification for the target. These options provide more flexibility for benchmarking targets in various environments, such as those with self-signed certificates or custom authentication mechanisms. The names were chosen to align with the existing --target argument to make it clear these apply to requests made to the target. The implementation now follows the following precedence for setting request headers: 1. Headers from the `--target-header` CLI option (highest priority). 2. Headers defined in a `.env` file or as environment variables. 3. Default headers derived from other parameters like `api_key`, `organization`, or `project` (lowest priority). This commit also adds documentation and tests for these new features, covering both CLI usage and configuration via environment variables or a `.env` file.
Created new guides for configuration options (environment variables, .env files) and supported data formats. It also updates the main CLI reference and links to the new guides.
Refactors the CLI arguments to move backend-specific options under the `--backend-args` flag. - Removes `--target-header` and `--target-skip-ssl-verify` from the CLI. - Updates the `OpenAIHTTPBackend` to parse `headers` and `verify` from the `backend_args` dictionary. - Renames `verify_ssl` to the protocol-neutral term `verify`. - Updates documentation and tests to reflect the new argument structure.
kdelee
force-pushed
the
ssl_verify_auth_header
branch
from
037cd42 to
e816a81
Compare
|
@sjmonson pushed some updates, looks like workflow needs approval |
sjmonson
requested changes
Jul 17, 2025
| @@ -94,6 +94,7 @@ def __init__( | |||
| extra_query: Optional[dict] = None, | |||
| extra_body: Optional[dict] = None, | |||
| remove_from_body: Optional[list[str]] = None, | |||
| **kwargs, | |||
There was a problem hiding this comment.
Follow same pattern as other optionals
Suggested change
| **kwargs, | |
| headers: Optional[dict] = None, | |
| verify: Optional[bool] = None, |
Comment on lines
+132
to
+134
| user_headers = kwargs.pop("headers", settings.openai.headers or {}) | ||
| default_headers.update(user_headers) | ||
| self.headers = default_headers |
There was a problem hiding this comment.
It would be nice to layer this per-key and support removing headers. Something like this:
Suggested change
| user_headers = kwargs.pop("headers", settings.openai.headers or {}) | |
| default_headers.update(user_headers) | |
| self.headers = default_headers | |
| default_headers.update(settings.openai.headers or {}) | |
| default_headers.update(headers) | |
| self.headers = {k: v for k, v in default_headers.items() if v is not None} |
This would also allow you to simplify the default_headers construction to something like:
default_headers = {
"Authorization": f"Bearer {api_key}" if api_key else settings.openai.bearer_token,
"OpenAI-Organization": self.organization,
"OpenAI-Project": project or settings.openai.project,
}| self.timeout = timeout if timeout is not None else settings.request_timeout | ||
| self.http2 = http2 if http2 is not None else settings.request_http2 | ||
| self.follow_redirects = ( | ||
| follow_redirects | ||
| if follow_redirects is not None | ||
| else settings.request_follow_redirects | ||
| ) | ||
| self.verify = kwargs.pop("verify", settings.openai.verify) |
There was a problem hiding this comment.
Due to above suggestion:
Suggested change
| self.verify = kwargs.pop("verify", settings.openai.verify) | |
| self.verify = verify or settings.openai.verify |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds command-line options to the "guidellm benchmark" command to support
custom authentication headers and skipping SSL verification when
communicating with the target system.
New options:
--target-header: Allows specifying a custom header(s) for the target.
--target-skip-ssl-verify: A flag to disable SSL certificate verification for the target.
These options provide more flexibility for benchmarking targets in
various environments, such as those with self-signed certificates or
custom authentication mechanisms. The names were chosen to align with
the existing --target argument to make it clear these apply to requests
made to the target.
The implementation now follows the following precedence for setting request headers:
--target-headerCLI option (highest priority)..envfile or as environment variables.api_key,organization, orproject(lowestpriority).
This commit also adds documentation and tests for these new features,
covering both CLI usage and configuration via environment variables or a
.envfile.