Skip to content
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

fix(model): predict method for url/base64 #26

Merged
merged 1 commit into from
Feb 24, 2022

Conversation

Phelan164
Copy link
Contributor

Because

  • there is an endpoint for inference with URL or base64

This commit

  • update the REST endpoint and message request

@Phelan164 Phelan164 changed the title fix(model): udpate predict method for url/base64 fix(model): update predict method for url/base64 Feb 22, 2022
@Phelan164 Phelan164 changed the title fix(model): update predict method for url/base64 fix(model): predict method for url/base64 Feb 22, 2022
@bochengyang
Copy link
Contributor

@Phelan164 please refer to pipeline-backend's proto file, users then can have a unified request for file/URL/base64 inference/predict

  1. The content of the request
  2. The inference/predict request

@Phelan164
Copy link
Contributor Author

@bochengyang I checked on pipeline-backend. However, there is one issue when unifying all message for both stream method and rest api.

  • bytes content is required for stream gRPC method but url/base64 is required for normal method.
    So from unified struct, it do not show which message field is required for each method

@xiaofei-du
Copy link
Member

@bochengyang I checked on pipeline-backend. However, there is one issue when unifying all message for both stream method and rest api.

  • bytes content is required for stream gRPC method but url/base64 is required for normal method.
    So from unified struct, it do not show which message field is required for each method

I think it makes sense to separate messages for stream and rest API. It won't confuse the user.

Copy link
Member

@xiaofei-du xiaofei-du left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@Phelan164 Phelan164 merged commit 9bd38a3 into main Feb 24, 2022
@Phelan164 Phelan164 deleted the support-url-base64-predict branch February 24, 2022 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants