Add UpdateApplication to WebApi #1228
Conversation
|
/cc @cakecatz |
khanhtc1202
force-pushed
the
update-application-impl
branch
from
9a6445c
to
e044b58
Compare
|
Code coverage for golang is
|
| @@ -56,6 +56,7 @@ service WebService { | |||
| rpc SyncApplication(SyncApplicationRequest) returns (SyncApplicationResponse) {} | |||
| rpc GetApplication(GetApplicationRequest) returns (GetApplicationResponse) {} | |||
| rpc GenerateApplicationSealedSecret(GenerateApplicationSealedSecretRequest) returns (GenerateApplicationSealedSecretResponse) {} | |||
| rpc UpdateApplication(UpdateApplicationRequest) returns (UpdateApplicationResponse) {} | |||
There was a problem hiding this comment.
nit: Move this to right beblow AddApplication.
| } | ||
|
|
||
| message UpdateApplicationResponse { | ||
| bool completed = 1; |
There was a problem hiding this comment.
Is this necessary? No error already means the operation was successfully completed.
There was a problem hiding this comment.
I don't have strong opinion about this 😅 let's remove the field 🆗
| }) | ||
| if err != nil { | ||
| a.logger.Error("failed to update application", zap.Error(err)) | ||
| return nil, status.Error(codes.Internal, "Failed to update application") |
There was a problem hiding this comment.
Not in the scope of this PR, but I found an issue around this error.
A validation check is done before putting into the database. So in some cases, this is not an Internal error.
https://github.com/pipe-cd/pipe/blob/master/pkg/datastore/deploymentstore.go#L136
Can you please raise an issue for this?
khanhtc1202
force-pushed
the
update-application-impl
branch
from
e044b58
to
1158fef
Compare
|
/approve |
|
This pull request is APPROVED by Approvers can cancel the approval by writing |
|
Code coverage for golang is
|
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #1219
Does this PR introduce a user-facing change?: