-
Notifications
You must be signed in to change notification settings - Fork 82
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
Migration of Python model kserve grpc testcase UI -> API #2155
Migration of Python model kserve grpc testcase UI -> API #2155
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Robocop found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Robot Results
|
Verified with Jenkins Build 2206 |
@@ -19,6 +19,10 @@ | |||
|
|||
*** Variables *** | |||
${PYTHON_MODEL_NAME}= python | |||
${EXPECTED_INFERENCE_GRPC_OUTPUT_PYTHON}= {"modelName":"python","modelVersion":"1","id":"1","outputs":[{"name":"OUTPUT0","datatype":"FP32","shape":["4"]},{"name":"OUTPUT1","datatype":"FP32","shape":["4"]}],"rawOutputContents":["AgAAAAAAAAAAAAAAAAAAAA==","AAQAAAAAAAAAAAAAAAAAAA=="]} |
Check warning
Code scanning / Robocop
Line is too long ({{ line_length }}/{{ allowed_length }}) Warning test
Set Project And Runtime runtime=${KSERVE_RUNTIME_REST_NAME} protocol=${PROTOCOL_GRPC} namespace=${test_namespace} | ||
... download_in_pvc=${DOWNLOAD_IN_PVC} model_name=${PYTHON_MODEL_NAME} | ||
... storage_size=100Mi memory_request=100Mi | ||
${requests}= Create Dictionary memory=1Gi |
Check notice
Code scanning / Robocop
{{ create_keyword }} can be replaced with VAR Note test
Remove File ${INFERENCESERVICE_FILLED_FILEPATH} | ||
Wait For Pods To Be Ready label_selector=serving.kserve.io/inferenceservice=${PYTHON_MODEL_NAME} | ||
... namespace=${test_namespace} | ||
${pod_name}= Get Pod Name namespace=${test_namespace} |
Check notice
Code scanning / Robocop
Variable '{{ name }}' is assigned but not used Note test
Verified with Jenkins Build 2306 |
@@ -196,10 +195,9 @@ Compile Inference Service YAML | |||
ELSE | |||
${mode}= Get KServe Default Deployment Mode From DSC | |||
END | |||
Log message=Using defaultDeploymentMode set in the DSC: ${mode} | |||
Log message=Using defaultDeploymentMode set in the DSC: ${mode}S |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what S
stands for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
by mistake added S
Log ${host} | ||
${inference_output}= Query Model With GRPCURL host=${host} port=443 | ||
... endpoint=inference.GRPCInferenceService/ModelInfer | ||
... json_body=@ input_filepath=${INFERENCE_GRPC_INPUT_PYTHONFILE} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why json_body=@
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needed @ for passing the input file for grpcurl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think you should be able to run this kw with json_body=${INFERENCE_GRPC_INPUT_PYTHONFILE}
and adding "@" in the INFERENCE_GRPC_INPUT_PYTHONFILE
, without input_filepath
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I already tried that approach, but it didn’t work. That’s why I’m using this method. We already discussed this in another PR earlier.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm that's very strange..thing is that as it is this keywork call does not make much sense with "json_body=@".
I can approve it since I don't think it's hurting or causing troubles, but i'd suggest to review the code and try to find the culprit
Quality Gate passedIssues Measures |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: bdattoma, Raghul-M, tarukumar 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 |
868ab73
into
red-hat-data-services:master
No description provided.