Skip to content
This repository was archived by the owner on Jul 4, 2025. It is now read-only.

Conversation

@louis-jan
Copy link
Contributor

@louis-jan louis-jan commented Oct 31, 2024

Describe Your Changes

  • When importing a model via API from Jan, the model's display name is extracted from the GGUF metadata but most of them are poorly defined. E.g. just model or blank space. This chore update allow clients to decide the model name. Also provided the missing API doc schema.
  1. When making a request to import a model, I should be able to provide it's name.
Screenshot 2024-10-31 at 22 05 27
  1. The imported model should have the name updated accordingly
Screenshot 2024-10-31 at 22 05 20
  1. I should be able to see Model Import API on the scalar API reference
Screenshot 2024-10-31 at 22 38 05

Changes made

  1. In models.cc:

    • A new variable modelName is extracted from the JSON request.
    • A check is added to throw an error if modelHandle is empty.
    • The name field of model_config is updated with modelName if it's not empty.
  2. In test_api_model_import.py:

    • The existing test case is slightly modified (changing json = body_json to json=body_json).
    • Three new test cases are added:
      a. Testing model import with a custom name (marked to skip by default).
      b. Testing model import with an invalid path (expected to fail).
      c. Testing model import with a missing model (expected to fail).
  3. A new endpoint /v1/models/import is added:

    • It's a POST request for importing a model.
    • The operation ID is "ModelsController_importModel".
    • It requires a request body with a schema referencing "#/components/schemas/ImportModelRequest".
    • The response for a successful import (200 status) includes a schema referencing "#/components/schemas/ImportModelResponse".
  4. Two new schemas are added:

    • ImportModelRequest:

      • Contains properties for "model" (identifier), "modelPath" (file path), and "name" (display name).
      • "model" and "modelPath" are required fields.
    • ImportModelResponse:

      • Contains properties for "message" (success message), "modelHandle" (unique identifier), and "result" (status).
      • All fields are required.
  5. Pull with a desired name

Screenshot 2024-11-01 at 10 39 27 Screenshot 2024-11-01 at 10 39 56
  1. Update Model Pull API doc
Screenshot 2024-11-01 at 10 45 18

These changes enhance the model import functionality by allowing custom names and adding more robust error checking. The new test cases improve the coverage of the API's behavior under different scenarios. Also provided the missing API doc, with appropriate request and response structures defined in the OpenAPI specification.

Self Checklist

  • Added relevant comments, esp in complex areas
  • Updated docs (for bug fixes / features)
  • Created issues for follow-up changes or refactoring needed

@github-actions
Copy link
Contributor

github-actions bot commented Oct 31, 2024

Preview URL: https://4e75f7b6.cortex-docs.pages.dev

@louis-jan louis-jan requested a review from a team October 31, 2024 16:07
Copy link
Contributor

@namchuai namchuai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ lgtm

@namchuai
Copy link
Contributor

However, it would be great if you can do the same for model pull api 🙏

@louis-jan
Copy link
Contributor Author

However, it would be great if you can do the same for model pull api 🙏

Ye adding here 🚀

@louis-jan louis-jan merged commit f5fbad6 into dev Nov 1, 2024
5 checks passed
@louis-jan louis-jan deleted the chore/import-model-with-custom-name branch November 1, 2024 04:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants