Add pagination for direct-helm get summaries. #3091
Conversation
| @@ -149,8 +150,12 @@ func (s *Server) GetAvailablePackageSummaries(ctx context.Context, request *core | |||
| cq.AppVersion = request.FilterOptions.AppVersion | |||
| } | |||
|
|
|||
| // TODO: We are not yet returning paginated results here | |||
| charts, _, err := s.manager.GetPaginatedChartListWithFilters(cq, 1, 0) | |||
| pageSize := request.GetPaginationOptions().GetPageSize() | |||
There was a problem hiding this comment.
Mmm, so this way is the most idiomatic way to check nulls? Interesting
There was a problem hiding this comment.
Yeah, though it's specific to the grpc message generation, which provides these getters that initialise to the default (empty) struct.
| }, nil | ||
| } | ||
|
|
||
| // pageOffsetFromPageToken converts a page token to an integer offset | ||
| // representing the page of results. | ||
| // TODO(mnelson): When aggregating results from different plugins, we'll |
There was a problem hiding this comment.
Great, so atm, it is just the same old behavior we already implemented.
There was a problem hiding this comment.
Yep. And we can leave it that way until we're updating to built-in cache support for plugins, I'd think.
| if tc.request.GetPaginationOptions().GetPageSize() > 0 { | ||
| mock.ExpectQuery("SELECT count"). | ||
| WithArgs(tc.request.Context.Namespace, tc.server.globalPackagingNamespace). | ||
| WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(3)) |
There was a problem hiding this comment.
Yeah, it's a query in the assetsvc that returns the total pages (so counts the filtered but not paginated set): https://github.com/kubeapps/kubeapps/blob/master/cmd/assetsvc/pkg/utils/postgresql_utils.go#L80
| @@ -149,8 +150,12 @@ func (s *Server) GetAvailablePackageSummaries(ctx context.Context, request *core | |||
| cq.AppVersion = request.FilterOptions.AppVersion | |||
| } | |||
|
|
|||
| // TODO: We are not yet returning paginated results here | |||
| charts, _, err := s.manager.GetPaginatedChartListWithFilters(cq, 1, 0) | |||
| pageSize := request.GetPaginationOptions().GetPageSize() | |||
There was a problem hiding this comment.
Yeah, though it's specific to the grpc message generation, which provides these getters that initialise to the default (empty) struct.
| }, nil | ||
| } | ||
|
|
||
| // pageOffsetFromPageToken converts a page token to an integer offset | ||
| // representing the page of results. | ||
| // TODO(mnelson): When aggregating results from different plugins, we'll |
There was a problem hiding this comment.
Yep. And we can leave it that way until we're updating to built-in cache support for plugins, I'd think.
| if tc.request.GetPaginationOptions().GetPageSize() > 0 { | ||
| mock.ExpectQuery("SELECT count"). | ||
| WithArgs(tc.request.Context.Namespace, tc.server.globalPackagingNamespace). | ||
| WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(3)) |
There was a problem hiding this comment.
Yeah, it's a query in the assetsvc that returns the total pages (so counts the filtered but not paginated set): https://github.com/kubeapps/kubeapps/blob/master/cmd/assetsvc/pkg/utils/postgresql_utils.go#L80
Signed-off-by: Michael Nelson <minelson@vmware.com>
Description of the change
Adds and tests pagination for direct helm plugin's GetAvailablePackageSummaries request.
Applicable issues