-
Notifications
You must be signed in to change notification settings - Fork 117
Make validate_credentials max_tokens configurable and bump SDK version #274
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -183,7 +183,11 @@ def validate_credentials(self, model: str, credentials: dict) -> None: | |||||
| endpoint_url += "/" | ||||||
|
|
||||||
| # prepare the payload for a simple ping to the model | ||||||
| data = {"model": credentials.get("endpoint_model_name", model), "max_tokens": 5} | ||||||
| validate_credentials_max_tokens = credentials.get("validate_credentials_max_tokens", 5) or 5 | ||||||
| data = { | ||||||
| "model": credentials.get("endpoint_model_name", model), | ||||||
| "max_tokens": validate_credentials_max_tokens, | ||||||
| } | ||||||
|
|
||||||
| completion_type = LLMMode.value_of(credentials["mode"]) | ||||||
|
|
||||||
|
|
@@ -201,8 +205,11 @@ def validate_credentials(self, model: str, credentials: dict) -> None: | |||||
| # ADD stream validate_credentials | ||||||
| stream_mode_auth = credentials.get("stream_mode_auth", "not_use") | ||||||
| if stream_mode_auth == "use": | ||||||
| stream_validate_max_tokens = credentials.get("validate_credentials_max_tokens") or 10 | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line has the same issue as in the non-streaming case. Using To handle this correctly while keeping it concise, you can use an assignment expression to check for
Suggested change
|
||||||
| data["stream"] = True | ||||||
| data["max_tokens"] = 10 | ||||||
| # default 10 (introduced in PR #93) to ensure streaming endpoints emit a token chunk; | ||||||
| # allow overriding via credentials when explicitly provided. | ||||||
| data["max_tokens"] = stream_validate_max_tokens | ||||||
| response = requests.post(endpoint_url, headers=headers, json=data, timeout=(10, 300), stream=True) | ||||||
| if response.status_code != 200: | ||||||
| raise CredentialsValidateFailedError( | ||||||
|
|
||||||
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.
The use of
or 5can lead to unexpected behavior. If a user providesvalidate_credentials_max_tokens: 0, this expression will evaluate to5, which is likely not the intended behavior as0can be a valid value for some APIs.A more robust way to handle this is to explicitly check for
Nonebefore applying the default. This can be done concisely using an assignment expression (walrus operator), which is supported by your target Python version (>=3.11).