Skip to content

Conversation

seanshi-scale
Copy link
Contributor

@seanshi-scale seanshi-scale commented Jun 27, 2022

Add traceback as a field on EndpointResponse for debugging purposes
Also fix unrelated table printing error
see sc-382133 and https://github.com/scaleapi/models/pull/3493

Example invocation:
sync_endpoint.predict(EndpointRequest(**broken_args))
which gives an EndpointResponse that looks like

status: FAILURE, result: None, result_url: None, traceback: Traceback (most recent call last): File "/app/hosted_model_inference/hosted_model_inference/inference/sync_inference/fastapi_server.py", line 92, in predict return run_predict(predict_fn, payload) File "/app/hosted_model_inference/hosted_model_inference/inference/common.py", line 195, in run_predict return predict_on_args(predict_fn, request_params.args, request_params.return_pickled) File "/app/hosted_model_inference/hosted_model_inference/inference/common.py", line 152, in predict_on_args output = predict_fn(**inputs) TypeError: predict_fn() got an unexpected keyword argument 'x' 
launch/client.py Outdated
resp = self.connection.post(
payload=payload,
route=f"{SYNC_TASK_PATH}/{endpoint_id}",
handle_bad_response=False,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

if we return http 200 + "failed" as part of the sync endpoint failing response, we can get rid of this "handle_bad_response" thing

@seanshi-scale seanshi-scale self-assigned this Jun 28, 2022
@seanshi-scale seanshi-scale marked this pull request as ready for review June 28, 2022 00:35
@seanshi-scale seanshi-scale changed the title Seanshi/task tracebacks Tracebacks from endpoint failures are propagated to client Jun 28, 2022
Copy link
Contributor

@syandroo syandroo left a comment

Choose a reason for hiding this comment

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

don't forget to bump verison also

return EndpointResponse(
client=self.client,
status=TASK_SUCCESS_STATE,
status=raw_response.get("state"),
Copy link
Contributor

Choose a reason for hiding this comment

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

should we consolidate on state vs status haha

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should tbh, I like status personally

@seanshi-scale seanshi-scale requested a review from syandroo June 29, 2022 02:39
Copy link
Member

@yixu34 yixu34 left a comment

Choose a reason for hiding this comment

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

Think this looks ok? Thanks for updating the docs.

Can you update the PR with an example invocation and stack trace? Just curious what it looks like (though we should be careful not to put anything sensitive).

@seanshi-scale seanshi-scale merged commit ad86471 into master Jun 29, 2022
@seanshi-scale seanshi-scale deleted the seanshi/task-tracebacks branch June 29, 2022 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants