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
Implement listing and getting piped for web #159
Conversation
The following files are not gofmt-ed. By commenting pkg/app/api/service/webservice/model.go--- pkg/app/api/service/webservice/model.go.orig
+++ pkg/app/api/service/webservice/model.go
@@ -23,15 +23,15 @@
return nil
}
return &Piped{
- Id: input.Id,
- Desc: input.Desc,
- ProjectId: input.ProjectId,
- Version: input.Version,
- StartedAt: input.StartedAt,
+ Id: input.Id,
+ Desc: input.Desc,
+ ProjectId: input.ProjectId,
+ Version: input.Version,
+ StartedAt: input.StartedAt,
CloudProviders: input.CloudProviders,
- RepositoryIds: input.RepositoryIds,
- Disabled: input.Disabled,
- CreatedAt: input.CreatedAt,
- UpdatedAt: input.UpdatedAt,
+ RepositoryIds: input.RepositoryIds,
+ Disabled: input.Disabled,
+ CreatedAt: input.CreatedAt,
+ UpdatedAt: input.UpdatedAt,
}
}
|
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.
}, nil | ||
} | ||
|
||
func (a *FakeWebAPI) GetPiped(ctx context.Context, req *webservice.GetPipedRequest) (*webservice.GetPipedResponse, error) { |
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.
ctx
is unused in GetPiped
}, nil | ||
} | ||
|
||
func (a *FakeWebAPI) GetPiped(ctx context.Context, req *webservice.GetPipedRequest) (*webservice.GetPipedResponse, error) { |
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.
req
is unused in GetPiped
/golinter fmt |
} | ||
|
||
// OmittedPiped is the minimal message for listing on the web application. | ||
message OmittedPiped { |
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.
Not sure about needing this. IMO just use Piped
is fine.
And we can add ConnectionStatus field to the above Piped
message too.
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.
About the size, I think we do not have a large number of piped
for each project.
And we are also returning a list of larger models such as deployment, applications.
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.
That's good. My brain doesn't work in today 😇
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.
👍 A long day.
"@org_golang_google_grpc//:go_default_library", | ||
"@org_golang_google_protobuf//reflect/protoreflect:go_default_library", |
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.
Not sure why this is included?
I could not find where we are using this.
@@ -92,9 +136,20 @@ message DisablePipedResponse { | |||
} | |||
|
|||
message ListPipedsRequest { | |||
// Whether to include the status value in the response message. | |||
bool withStatus = 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.
nit: with_status
pkg/app/api/api/web_api.go
Outdated
Disabled: pipeds[i].Disabled, | ||
CreatedAt: pipeds[i].CreatedAt, | ||
UpdatedAt: pipeds[i].UpdatedAt, | ||
} |
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.
Now we can use webserivce.MakePiped
function.
The following ISSUES will be created once got merged. If you want me to skip creating the issue, you can use Details1. While reducing the number of query and get ping connection status from piped_statsThis was created by todo plugin since "TODO:" was found in 1854dad when #159 was merged. cc: @stormcat24. |
Thank you. |
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?: