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.

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

Updates core installed pkg summary handler for pagination. #4719

Merged
merged 2 commits into from
May 19, 2022

Conversation

absoludity
Copy link
Contributor

@absoludity absoludity commented May 17, 2022

Signed-off-by: Michael Nelson [email protected]

Description of the change

Follows on from #4694 , applying the same functionality to the core GetInstalledPackageSummaries handler.

It does not yet work, because the dashboard client is currently assuming knowledge of the pagination token, so I need to do a separate PR for the dashboard to remove that assumption for GetInstalledPackageSummaries, as I did earlier for the available packages in the dashboard (hence being a draft). EDIT: it ended up only requiring a 2-line change to the dashboard so I've just included it in this PR.

Works to display correlated installed package summaries:
installed-apps

Benefits

Core pagination for multiple plugins will display installed packages in order efficiently.

Possible drawbacks

Applicable issues

Additional information

I did play a little with generics at first but couldn't yet see a useful way to re-use the functionality due to (a) small differences between the responses - available package summaries including categories, for example, and (b) the inability to define a shared interface because go doesn't let you add functions to a struct from another package (and I'm not sure whether we want to add them to the generated packages). Either way, I've left a TODO for us to do this in the future if we want.

Base automatically changed from 3399-core-pagination-6 to main May 18, 2022 00:45
@netlify
Copy link

netlify bot commented May 18, 2022

Deploy Preview for kubeapps-dev canceled.

Name Link
🔨 Latest commit 2884707
🔍 Latest deploy log https://app.netlify.com/sites/kubeapps-dev/deploys/628588ab94054600097d70c4

@absoludity absoludity force-pushed the 3399-core-pagination-7 branch from 45b9513 to d5b93d7 Compare May 18, 2022 01:43
@absoludity absoludity marked this pull request as ready for review May 18, 2022 01:44
Copy link
Contributor

@antgamdia antgamdia left a comment

Choose a reason for hiding this comment

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

Awesome! That's great we finally have pagination in both get X summaries requests! Thanks for the good work here!

I've noticed the watermark behind some packages is the default one (kubeapps mark), instead of using the plugin. Perhaps it gets fixed with #4728:

image

@absoludity absoludity force-pushed the 3399-core-pagination-7 branch from d5b93d7 to 2884707 Compare May 19, 2022 00:00
@absoludity absoludity merged commit c7e64e5 into main May 19, 2022
@absoludity absoludity deleted the 3399-core-pagination-7 branch May 19, 2022 01:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants