-
Notifications
You must be signed in to change notification settings - Fork 23
Edit FunctionMetadataResponses message to be more descriptive #59
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
Conversation
src/proto/FunctionRpc.proto
Outdated
| message FunctionMetadataResponses { | ||
| // list of function indexing responses | ||
| repeated FunctionLoadRequest results = 1; | ||
| repeated FunctionLoadRequest function_load_requests = 1; |
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 this is the simplest description/name. It's clearer than "results" and there is no confusion on the meaning b/c almost all messages with a "result" field refer to the StatusResult.
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.
function_load_requests_results to be more clear, to ensure this is the result of function load on language worker
|
|
||
| // status of overall metadata request | ||
| StatusResult overall_status = 2; | ||
| StatusResult result = 2; |
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.
Consistency w/ other messages - StatusResult type is generally returned in "result".
pragnagopa
left a comment
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.
added a minor comment
Added changes to the FunctionMetadataResponses message to be more consistent with the design of other messages and to be more descriptive.