-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
update kfservice status URL #289
Conversation
/cc @yuzisun Could you please help to take a look? Thanks. |
Sounds good to me, want to hear @johnugeorge’s thoughts. |
Looks good to me. |
/lgtm |
Discussed with Ellis we can always extend if needed. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: yuzisun The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
#### Motivation The file containing the default configuration for the ModelMesh Controller is called `config-defaults.yaml`, which resides in a directory also called `config-defaults.yaml` as pointed out by @adrwong-lora in issue kserve#205 ``` /etc/model-serving/config-defaults.yaml/config-defaults.yaml ``` #### Modifications Change `mountPath` from `/etc/model-serving/config-defaults.yaml` to `/etc/model-serving/config/default` #### Result The file containing the default configuration file after this fix: ``` /etc/model-serving/config/default/config-defaults.yaml ``` Which more closely reflects the path of the [`config-defaults.yaml`](https://github.com/kserve/modelmesh-serving/blob/main/config/default/config-defaults.yaml) file in the `modelmesh-serving` repository: ``` config/default/config-defaults.yaml ``` Resolves kserve#205 /cc @njhill /fyi @adrwong-lora Signed-off-by: Christian Kadner <ckadner@us.ibm.com>
What this PR does / why we need it:
Background: After introducing
apis.URL
to status in the #251, we have to create this hack since that struct is deep in go standard net library and we can not add openapi annotation, @yuzisun suggests creating our simplified URL struct. After investigation, I think that's better to back tostring
for URL since if we ceate our URL struct, that's hard to assign with net.url (cannot sure no issue in the future). And there is a String function in the knative.dev/pkg/apis.URL, we can use that and transfer to string for kfservice. That's good enough for kfservice.See more details in the #272
This change is