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.

Sign up for GitHub

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

Jump to bottom

clean grpc response #161

Merged
merged 22 commits into from
Apr 13, 2023
Merged

clean grpc response #161

merged 22 commits into from
Apr 13, 2023

Conversation

sakamoto-souta
Copy link
Collaborator

Summary

clean grpc response

Purpose

avoid having content in the return value that duplicates the content of the error.

Contents

erase message and is_ok in the return value of responses

Testing Methods Performed

make test t=./manage_container p=medium

arukuka
arukuka requested changes Apr 12, 2023
View reviewed changes
Copy link
Member

@arukuka arukuka left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution!
I have commented on some points that I am concerned, so please check it out.

packages/server/manage_container/server/libc_to_manage_container/L2m.go Outdated Show resolved Hide resolved
packages/server/manage_container/server/libc_to_manage_container/L2m.go Show resolved Hide resolved
packages/server/manage_container/server/libc_to_manage_container/L2m.go Show resolved Hide resolved
packages/server/manage_container/server/libc_to_manage_container/L2m.go Outdated Show resolved Hide resolved
packages/client/libclient-py/quickmpc/qmpc_server.py Outdated Show resolved Hide resolved
KotaTakahashi9320
KotaTakahashi9320 requested changes Apr 12, 2023
View reviewed changes
Copy link
Collaborator

@KotaTakahashi9320 KotaTakahashi9320 left a comment

Choose a reason for hiding this comment

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

Thank you for contribution.
I commented.

proto/libc_to_manage_container/libc_to_manage.proto Outdated Show resolved Hide resolved
@sakamoto-souta sakamoto-souta force-pushed the feature/sakamoto/clean_gRPC_response branch from 2216763 to dc59aa0 Compare April 12, 2023 04:56
KotaTakahashi9320
KotaTakahashi9320 approved these changes Apr 12, 2023
View reviewed changes
Copy link
Collaborator

@KotaTakahashi9320 KotaTakahashi9320 left a comment

Choose a reason for hiding this comment

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

LGTM

arukuka
arukuka reviewed Apr 12, 2023
View reviewed changes
Copy link
Member

@arukuka arukuka left a comment

Choose a reason for hiding this comment

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

There were some parts I hadn't checked thoroughly 🙇,
please check the comments.

packages/server/manage_container/server/libc_to_manage_container/L2m.go Outdated Show resolved Hide resolved
packages/server/manage_container/server/libc_to_manage_container/L2m.go Outdated Show resolved Hide resolved
arukuka
arukuka approved these changes Apr 12, 2023
View reviewed changes
Copy link
Member

@arukuka arukuka left a comment

Choose a reason for hiding this comment

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

Thank you!

@sakamoto-souta sakamoto-souta force-pushed the feature/sakamoto/clean_gRPC_response branch from f6973c3 to 5305a2e Compare April 12, 2023 07:37
@arukuka
Copy link
Member

arukuka commented Apr 12, 2023

@sakamoto-souta
Please rebase this branch to latest main branch: 5d0d080 🙇

@sakamoto-souta
FIX: make sendshare return only error
a19afec
@sakamoto-souta
FIX: correspond the case of response without is_ok
bd0a185
@sakamoto-souta
FIX: fix type of return value of local SendShare
44d5fcc
@sakamoto-souta
FIX: type of SendShare in proto
d0f467d
@sakamoto-souta
FIX: return value of DeleteShare
d73ef01
@sakamoto-souta
FIX: return value of GetElapsedTime
200d1b6
@sakamoto-souta
FIX: return value of GetJobErrorInfo
ea9aad9
@sakamoto-souta
FIX: return value of GetDatalist
ce56ca1
@sakamoto-souta
FIX: return value of GetSchema
78834f9
@sakamoto-souta
FIX: return value of ExecuteComputation
08939e8
@sakamoto-souta
FIX: type of Response in proto
44720d1
@sakamoto-souta
FIX: fix types of return values in local_server.py
0e4dead
@sakamoto-souta
FIX: regenerate proto
278df5d
@sakamoto-souta
FIX: change message to _
32e4b28
@sakamoto-souta
FIX: import empty_pb2
e6da5db
@sakamoto-souta
FIX: does not check is_ok
e7cfc62
@sakamoto-souta
FIX: return nil, err when error occurs
f7f7a14
@sakamoto-souta
FIX: show error content to client
d5d637e
@sakamoto-souta
DELETE: check is_ok
70d8d28
@sakamoto-souta
DEL: is_ok and message from stream
6e8c45d
@sakamoto-souta sakamoto-souta force-pushed the feature/sakamoto/clean_gRPC_response branch from 5305a2e to 5168fdb Compare April 12, 2023 10:14
@arukuka arukuka merged commit 41a35a5 into main Apr 13, 2023
@arukuka arukuka deleted the feature/sakamoto/clean_gRPC_response branch April 13, 2023 01:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KotaTakahashi9320 KotaTakahashi9320 KotaTakahashi9320 approved these changes

@arukuka arukuka arukuka approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sakamoto-souta @arukuka @KotaTakahashi9320