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

Use iterators and discourage explicit pagination #100

Open
jennydaman opened this issue Oct 7, 2023 · 0 comments
Open

Use iterators and discourage explicit pagination #100

jennydaman opened this issue Oct 7, 2023 · 0 comments

Comments

@jennydaman
Copy link
Contributor

Getters accept limit and offset parameters, which does not encourage the client to implement API pagination properly. For example, in the codebase for ChRIS_ui and chris_ui_ssr we see in many places where {limit: 100*} is given as a parameter.

$ grep -Fnr 'limit: 100' ChRIS_ui/src chris_ui_ssr/src
ChRIS_ui/src/api/common/index.ts:203:    limit: 100,
ChRIS_ui/src/api/common/index.ts:220:    limit: 1000,
ChRIS_ui/src/components/catalog/PluginCatalog.tsx:54:            const plugins = await plugin.getPlugins({ limit: 1000 });
ChRIS_ui/src/components/feed/AddNode/GuidedConfig.tsx:63:        limit: 1000,
ChRIS_ui/src/components/feed/AddPipeline/AddPipeline.tsx:82:            limit: 1000,
ChRIS_ui/src/components/feed/AddTsNode/index.tsx:33:        limit: 1000,
ChRIS_ui/src/components/feed/CreateFeed/utils/pipelines.ts:8:    limit: 100,
ChRIS_ui/src/components/feed/CreateFeed/utils/pipelines.ts:25:    limit: 1000,
ChRIS_ui/src/components/feed/NodeDetails/NodeDetails.tsx:61:        limit: 100,
ChRIS_ui/src/components/feed/NodeDetails/NodeDetails.tsx:67:        limit: 100,
ChRIS_ui/src/components/feed/Pipelines/GeneralCompute.tsx:25:        limit: 100,
ChRIS_ui/src/components/feed/Pipelines/ConfigurationPage.tsx:50:          limit: 1000,
ChRIS_ui/src/pages/DataLibrary/components/UserLibrary/BrowserContainer.tsx:122:      limit: 100,
ChRIS_ui/src/pages/DataLibrary/components/UserLibrary/index.tsx:144:          limit: 1000,
ChRIS_ui/src/pages/DataLibrary/components/UserLibrary/index.tsx:268:            limit: 1000,
ChRIS_ui/src/pages/SinglePluginPage/SinglePlugin.tsx:46:        limit: 1000,
ChRIS_ui/src/store/resources/saga.ts:42:  const params = { limit: 100, offset: 0 };
chris_ui_ssr/src/lib/utilities/library/index.ts:44:		limit: 10000000,
chris_ui_ssr/src/routes/(_main)/data/[...slug]/+page.server.ts:37:		limit: 1000000,
chris_ui_ssr/src/routes/api/downloads/+server.ts:25:		limit: 100000
chris_ui_ssr/src/routes/api/metadata/+server.ts:19:	const files = await pathList.getFiles({ limit: 100000 });

This practice causes performance problems and code quality issues (pagination parameters are often specified in the code where pagination is not relevant in the context)

My suggestion is to provide an iterator (async iterable, stream, whatever it's called) based interface similar to how it is implemented in aiochris and the rust chris client.

https://fnndsc.github.io/aiochris/v0.4.0/aiochris.html#Search

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

No branches or pull requests

1 participant