Skip to content

runtime: accept a config object in the download endpoints - #1977

Open
Daksha1611 wants to merge 1 commit into
volcano-sh:mainfrom
Daksha1611:fix/runtime-object-config
Open

Daksha1611 wants to merge 1 commit into
volcano-sh:mainfrom
Daksha1611:fix/runtime-object-config

Conversation

@Daksha1611

Copy link
Copy Markdown

What type of PR is this?

/kind bug

What this PR does / why we need it:

/v1/download_model and /v1/load_lora_adapter document config as a JSON object, but any non-empty object returns HTTP 500 because load_config passes it to json.loads.

This PR makes load_config accept a dict as well as a JSON string. The CLI continues to pass a string. The dict is copied so that the request body is not modified. Values that are not objects now raise ValueError with a clear message.

Which issue(s) this PR fixes:
Fixes #1976

Bug evidence (required for bug-related PRs):

Requests sent over HTTP to the runtime server started with python -m kthena.runtime.app, using a local pvc:// source.

config value Before (3809f22) After
{} 200 200
{"hf_token":"dummy"} 500, nothing downloaded 200, file downloaded
"{\"hf_token\":\"dummy\"}" 200 200
123 500, the JSON object must be str, bytes or bytearray, not int 500, Invalid configuration JSON format.

Error before the fix:

{"detail":"Download failed: the JSON object must be str, bytes or bytearray, not dict"}

This was reproduced locally and not on a cluster. /v1/load_lora_adapter was covered through the FastAPI test client rather than a running server.

Special notes for your reviewer:

A value that is not an object still returns HTTP 500 because the handlers map every exception to 500. Returning 400 would be a separate change, and I have kept it out of this PR.

Testing: pytest python/kthena/tests (125 passed, Python 3.12) and ruff check . --config python/kthena/pyproject.toml. Eight of the new tests fail without the fix.

Does this PR introduce a user-facing change?:

Fixed the runtime download endpoints returning HTTP 500 when `config` is a non-empty JSON object.

Signed-off-by: Daksha1611 <mehtadaksha1611@gmail.com>
@volcano-sh-bot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign hzxuzhonghu for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@volcano-sh-bot

Copy link
Copy Markdown
Contributor

Welcome @Daksha1611! It looks like this is your first PR to volcano-sh/kthena 🎉

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Accepted

Development

Successfully merging this pull request may close these issues.

runtime: /v1/download_model and /v1/load_lora_adapter return 500 for a non-empty config object

2 participants