-
Notifications
You must be signed in to change notification settings - Fork 100
feat: add meaningful LMStudioPredictionError subclasses and session-s… #136
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?
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 |
---|---|---|
@@ -0,0 +1,70 @@ | ||
# This workflow will upload a Python Package to PyPI when a release is created | ||
# For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-python#publishing-to-package-registries | ||
|
||
# This workflow uses actions that are not certified by GitHub. | ||
# They are provided by a third-party and are governed by | ||
# separate terms of service, privacy policy, and support | ||
# documentation. | ||
|
||
name: Upload Python Package | ||
|
||
on: | ||
release: | ||
types: [published] | ||
|
||
permissions: | ||
contents: read | ||
|
||
jobs: | ||
release-build: | ||
runs-on: ubuntu-latest | ||
|
||
steps: | ||
- uses: actions/checkout@v4 | ||
|
||
- uses: actions/setup-python@v5 | ||
with: | ||
python-version: "3.x" | ||
|
||
- name: Build release distributions | ||
run: | | ||
# NOTE: put your own distribution build steps here. | ||
python -m pip install build | ||
python -m build | ||
- name: Upload distributions | ||
uses: actions/upload-artifact@v4 | ||
with: | ||
name: release-dists | ||
path: dist/ | ||
|
||
pypi-publish: | ||
runs-on: ubuntu-latest | ||
needs: | ||
- release-build | ||
permissions: | ||
# IMPORTANT: this permission is mandatory for trusted publishing | ||
id-token: write | ||
|
||
# Dedicated environments with protections for publishing are strongly recommended. | ||
# For more information, see: https://docs.github.com/en/actions/deployment/targeting-different-environments/using-environments-for-deployment#deployment-protection-rules | ||
environment: | ||
name: pypi | ||
# OPTIONAL: uncomment and update to include your PyPI project URL in the deployment status: | ||
# url: https://pypi.org/p/YOURPROJECT | ||
# | ||
# ALTERNATIVE: if your GitHub Release name is the PyPI project version string | ||
# ALTERNATIVE: exactly, uncomment the following line instead: | ||
# url: https://pypi.org/project/YOURPROJECT/${{ github.event.release.name }} | ||
|
||
steps: | ||
- name: Retrieve release distributions | ||
uses: actions/download-artifact@v4 | ||
with: | ||
name: release-dists | ||
path: dist/ | ||
|
||
- name: Publish release distributions to PyPI | ||
uses: pypa/gh-action-pypi-publish@release/v1 | ||
with: | ||
packages-dir: dist/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
{ | ||
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. Editor specific config shouldn't be added to the repo. (I have |
||
"version": "2.0.0", | ||
"tasks": [ | ||
{ | ||
"type": "shell", | ||
"label": "Run chatbot example", | ||
"command": "python3 examples/chatbot.py" | ||
} | ||
] | ||
} |
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -92,7 +92,8 @@ addopts = "--strict-markers" | |||
markers = [ | ||||
"slow: marks tests as slow (deselect with '-m \"not slow\"')", | ||||
"lmstudio: marks tests as needing LM Studio (deselect with '-m \"not lmstudio\"')", | ||||
"wip: marks tests as a work-in-progress (select with '-m \"wip\"')" | ||||
"wip: marks tests as a work-in-progress (select with '-m \"wip\"')", | ||||
"asyncio: marks tests as asyncio-based", | ||||
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.
Suggested change
|
||||
] | ||||
# Warnings should only be emitted when being specifically tested | ||||
filterwarnings = [ | ||||
|
@@ -102,7 +103,6 @@ filterwarnings = [ | |||
log_format = "%(asctime)s %(levelname)s %(message)s" | ||||
log_date_format = "%Y-%m-%d %H:%M:%S" | ||||
# Each async test case gets a fresh event loop by default | ||||
asyncio_default_fixture_loop_scope = "function" | ||||
|
||||
[tool.coverage.run] | ||||
relative_files = true | ||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -17,6 +17,14 @@ | |||||||||||||||||
from .schemas import * | ||||||||||||||||||
from .history import * | ||||||||||||||||||
from .json_api import * | ||||||||||||||||||
from .json_api import ( | ||||||||||||||||||
LMStudioPredictionError, | ||||||||||||||||||
LMStudioModelLoadError, | ||||||||||||||||||
LMStudioInputValidationError, | ||||||||||||||||||
LMStudioPredictionTimeoutError, | ||||||||||||||||||
LMStudioPredictionCancelledError, | ||||||||||||||||||
LMStudioPredictionRuntimeError, | ||||||||||||||||||
) | ||||||||||||||||||
Comment on lines
+20
to
+27
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.
Suggested change
Exporting symbols at the top level is handled by listing them in the relevant |
||||||||||||||||||
from .async_api import * | ||||||||||||||||||
from .sync_api import * | ||||||||||||||||||
|
||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -418,10 +418,32 @@ def __init__(self, message: str) -> None: | |||
super().__init__(message, None) | ||||
|
||||
|
||||
@sdk_public_type | ||||
|
||||
Comment on lines
+421
to
+422
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.
Suggested change
Duplicated decorator |
||||
@sdk_public_type | ||||
class LMStudioPredictionError(LMStudioServerError): | ||||
"""Problems reported by the LM Studio instance during a model prediction.""" | ||||
|
||||
@sdk_public_type | ||||
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. The more granular subclasses and the session scoped fixture loading would be easier to follow if placed in different PRs. |
||||
class LMStudioModelLoadError(LMStudioPredictionError): | ||||
"""Raised when a model fails to load for a prediction.""" | ||||
|
||||
@sdk_public_type | ||||
class LMStudioInputValidationError(LMStudioPredictionError): | ||||
"""Raised when input to a prediction is invalid (e.g., bad prompt, bad parameters).""" | ||||
|
||||
@sdk_public_type | ||||
class LMStudioPredictionTimeoutError(LMStudioPredictionError): | ||||
"""Raised when a prediction times out before completion.""" | ||||
|
||||
@sdk_public_type | ||||
class LMStudioPredictionCancelledError(LMStudioPredictionError): | ||||
"""Raised when a prediction is cancelled before completion.""" | ||||
|
||||
@sdk_public_type | ||||
class LMStudioPredictionRuntimeError(LMStudioPredictionError): | ||||
"""Raised for unexpected runtime errors during prediction.""" | ||||
|
||||
|
||||
@sdk_public_type | ||||
class LMStudioClientError(LMStudioError): | ||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,7 @@ | |
|
||
from lmstudio import AsyncClient, EmbeddingLoadModelConfig, LMStudioModelNotFoundError | ||
|
||
from ..support import ( | ||
from tests.support import ( | ||
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. The use of relative imports is intentional and shouldn't be changed. |
||
EXPECTED_EMBEDDING, | ||
EXPECTED_EMBEDDING_CONTEXT_LENGTH, | ||
EXPECTED_EMBEDDING_ID, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,5 +2,27 @@ | |
|
||
import pytest | ||
|
||
|
||
# Ensure support module assertions provide failure details | ||
pytest.register_assert_rewrite("tests.support") | ||
|
||
# Session-scoped fixture for required model loading | ||
import asyncio | ||
import sys | ||
|
||
@pytest.fixture(scope="session", autouse=True) | ||
def load_required_models(): | ||
"""Load required models at the start of the test session.""" | ||
# Only run if LM Studio is accessible | ||
try: | ||
from tests.load_models import reload_models | ||
asyncio.run(reload_models()) | ||
except Exception as e: | ||
print(f"[Fixture] Skipping model loading: {e}", file=sys.stderr) | ||
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. Some environments consider printing anything to stderr to be a test failure in its own right, so ideally we wouldn't even try to load the models when This suggests that rather than def pytest_itemcollected(item):
if item.get_closest_marker("lmstudio") is not None:
item.applymarker(pytest.mark.usefixtures("load_required_models")) |
||
yield | ||
# Optionally unload models at the end of the session | ||
try: | ||
from tests.unload_models import unload_models | ||
asyncio.run(unload_models()) | ||
except Exception as e: | ||
print(f"[Fixture] Skipping model unloading: {e}", file=sys.stderr) |
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.
We have a
publish.yml
workflow, no need for this one.