fix api scheme #674
fix api scheme #674
Conversation
| // NOTE: json tags are required. Any new fields you add must have json tags for the fields to be serialized. | ||
|
|
||
| // ModelAdapterSpec defines the desired state of ModelAdapter | ||
| type ModelAdapterSpec struct { | ||
|
|
||
| // BaseModel is the identifier for the base model to which the ModelAdapter will be attached. | ||
| // +optional | ||
| BaseModel string `json:"baseModel,omitempty"` | ||
| BaseModel string `json:"baseModel"` |
There was a problem hiding this comment.
The original design here is to refer a model. but we didn't add the model CRD here (due to the reason I synced with you earlier).
I feel there're two options
- Once we have the model api, baseModel can ref that object.
- at this moment, we probably can leverage PodSelector or objectRef to upgrade the
PodSelector.
one of above should be specified
This would be a design discussion and I would like to know your thoughts here
There was a problem hiding this comment.
I didn't see a strong relation between the base model and the workload(pod), we can set the base model arbitrarily right now IIUC. And for the same base model like Qwen-1.5B-Instruct, we can have different kinds of service using the same foundation model, I didn't quite get the intention of this field.
Let me make this an optional field for now, we can revisit this at any time later. But since it's optional, it should be a pointer.
There was a problem hiding this comment.
yeah, right now, it could be arbitrary strings.
And for the same base model like Qwen-1.5B-Instruct, we can have different kinds of service using the same foundation model, I didn't quite get the intention of this field.
If the model you mentioned here is not linked to any kubernetes object, then I think baseModel will be confusing. The original intention here is to bind to a logical model concept. It could be a model CRD or a deployment with model identifier.
Lora1 could bind to deployment 1 actually using Qwen-1.5B-Instruct. lora2 could bind to deployment 2 using Qwen-1.5B-Instruct as well. If we have model crd which represents collection of 1 or many deployments, that the scope becomes larger. Technically, I think model concept doesn't exist in the system yet. so it's a little bit flexible here. We can do some offline discussion on this topic.
There was a problem hiding this comment.
make sense, I'll update the lint issue.
|
This for improving the quality! Excellent. I left some comments and we can have some discussion later |
|
@kerthcet please help address the linter issues and let's merge this change today. I plan to cut a release and we can stabilize the api a little bit |
finished. |
|
@kerthcet I fixed some e2e failures in main branch. Could you rebase the main branch code changes and submit again? |
Signed-off-by: kerthcet <kerthcet@gmail.com>
Signed-off-by: kerthcet <kerthcet@gmail.com>
Signed-off-by: kerthcet <kerthcet@gmail.com>
c0e5142 to
5a50b00
Compare
|
Seems failed by |
|
I think it's not related. |
|
Record the err msg: e2e_test.go:107: |
* fix api scheme Signed-off-by: kerthcet <kerthcet@gmail.com> * Address comments Signed-off-by: kerthcet <kerthcet@gmail.com> * Fix pointer might be nil error Signed-off-by: kerthcet <kerthcet@gmail.com> --------- Signed-off-by: kerthcet <kerthcet@gmail.com>
* fix api scheme Signed-off-by: kerthcet <kerthcet@gmail.com> * Address comments Signed-off-by: kerthcet <kerthcet@gmail.com> * Fix pointer might be nil error Signed-off-by: kerthcet <kerthcet@gmail.com> --------- Signed-off-by: kerthcet <kerthcet@gmail.com> Signed-off-by: Varun Gupta <varungup90@gmail.com>
Pull Request Description
[Please provide a clear and concise description of your changes here]
Roughly go through the APIs and found some places need to be polished. Not all of them to avoid code conflict.
Related Issues
Resolves: #[Insert issue number(s)]
Important: Before submitting, please complete the description above and review the checklist below.
Contribution Guidelines (Expand for Details)
We appreciate your contribution to aibrix! To ensure a smooth review process and maintain high code quality, please adhere to the following guidelines:
Pull Request Title Format
Your PR title should start with one of these prefixes to indicate the nature of the change:
[Bug]: Corrections to existing functionality[CI]: Changes to build process or CI pipeline[Docs]: Updates or additions to documentation[API]: Modifications to aibrix's API or interface[CLI]: Changes or additions to the Command Line Interface[Misc]: For changes not covered above (use sparingly)Note: For changes spanning multiple categories, use multiple prefixes in order of importance.
Submission Checklist
By submitting this PR, you confirm that you've read these guidelines and your changes align with the project's contribution standards.