models-web-app icon indicating copy to clipboard operation
models-web-app copied to clipboard

Published 20 hours ago • verified publisher icon kserve

Fix usage of custom ServingRuntimes

Open disrupted opened this issue 2 years ago • 12 comments

Fixes #46

disrupted avatar Nov 07 '22 14:11 disrupted

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: disrupted To complete the pull request process, please assign kimwnasptd after the PR has been reviewed. You can assign the PR to them by writing /assign @kimwnasptd in a comment when ready.

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

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

kserve-oss-bot avatar Nov 07 '22 14:11 kserve-oss-bot

hi @disrupted,

when I deployed the paddle inferecenservice with KServe, the kubeflow models-web-app had the display problem and had the error as following figure,

WechatIMG255

I think there is not the paddle servingruntime, and return None to cause the error. When I add the 'paddle' PredictType, patch and rebuild the models-web-app image, the problem can be fixed and the display become correctly, there is my changes,

https://github.com/harperjuanl/models-web-app

I also test your method, it does work too. I see it returns the predictor.model and remove the judgement. Does it have the problem that some frameworks are returned, which do not be supported by KServe?

harperjuanl avatar Nov 30 '22 08:11 harperjuanl

hi @harperjuanl,

thanks for your feedback, as well as verifying the issue & potential fix. Perhaps you're right, that this could lead to some issues with other frameworks, however I thought that ServingRuntimes are always specific to KServe and not used in other scenarios.

Adding custom ServingRuntimes (such as paddle in your case) to the known list of predictor types seems like a reasonable approach as well. But this would be better if it could be done through the KServe configuration, rather than having to patch the code, as it is the case in your example.

disrupted avatar Nov 30 '22 10:11 disrupted

@disrupted Please rebase to master

juliusvonkohout avatar Jun 26 '24 12:06 juliusvonkohout

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: disrupted Once this PR has been reviewed and has the lgtm label, please assign juliusvonkohout for approval by writing /assign @juliusvonkohout in a comment. For more information see:The Kubernetes Code Review Process.

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

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

oss-prow-bot[bot] avatar Jun 26 '24 12:06 oss-prow-bot[bot]

@disrupted Please rebase to master

done

disrupted avatar Jun 26 '24 12:06 disrupted

@disrupted you also have to sign the DCO to pass all tests.

juliusvonkohout avatar Jun 26 '24 12:06 juliusvonkohout

@disrupted you also have to sign the DCO to pass all tests.

done

disrupted avatar Jun 26 '24 14:06 disrupted

/assign @juliusvonkohout

disrupted avatar Jun 27 '24 08:06 disrupted

@disrupted the frontend test is not passing. Maybe it is broken.

juliusvonkohout avatar Jun 29 '24 19:06 juliusvonkohout

@disrupted the frontend test is not passing. Maybe it is broken.

it appears to me that the error is unrelated to the changes made in this PR

disrupted avatar Jul 02 '24 10:07 disrupted

@disrupted the frontend test is not passing. Maybe it is broken.

it appears to me that the error is unrelated to the changes made in this PR

You might be right, but we still have to fix the tests either way.

juliusvonkohout avatar Jul 03 '24 14:07 juliusvonkohout

merged in #92

juliusvonkohout avatar Sep 03 '24 11:09 juliusvonkohout