Skip to content
This repository has been archived by the owner on Jul 25, 2022. It is now read-only.

GH-2268 Update the limit param in graphql api's #2269

Merged
merged 5 commits into from
Sep 24, 2020
Merged

GH-2268 Update the limit param in graphql api's #2269

merged 5 commits into from
Sep 24, 2020

Conversation

johnsonw
Copy link
Contributor

@johnsonw johnsonw commented Sep 23, 2020

Fixes #2268.

Currently, the limit param allows you to specify a limit when retrieving items back from an API call. This is a numeric value that can be passed in but in some cases, we may want all results. To address this, postress allows us to pass in ALL instead of a numeric value, which is effectively the same as not passing a limit at all. Update all of the graphql API endpoints such that if None is passed in it will pass ALL to the LIMIT clause in the query. This will allow us to get all entries without modifying the existing api.

Signed-off-by: johnsonw wjohnson@whamcloud.com


This change is Reviewable

@johnsonw johnsonw added this to the IML EX V3 milestone Sep 23, 2020
@johnsonw johnsonw requested a review from a team September 23, 2020 19:35
@johnsonw johnsonw self-assigned this Sep 23, 2020
iml-api/src/graphql.rs Outdated Show resolved Hide resolved
Copy link
Member

@jgrund jgrund left a comment

Choose a reason for hiding this comment

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

Comments need updating as well, as they still say they default to 20.

limit.unwrap_or(20) as i64
limit
.map(|x| x.to_string())
.unwrap_or_else(|| "ALL".to_string()) as String
Copy link
Member

Choose a reason for hiding this comment

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

as String really needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It required the cast.

Fixes #2268.

Currently, the limit param allows you to specify a limit when retrieving items back from an API call. This is a numeric value that can be passed in but in some cases, we may want all results. To address this, postress allows us to pass in ALL instead of a numeric value, which is effectively the same as not passing a limit at all. Update all of the graphql API endpoints such that if None is passed in it will pass ALL to the LIMIT clause in the query. This will allow us to get all entries without modifying the existing api.

Signed-off-by: johnsonw <wjohnson@whamcloud.com>
Signed-off-by: johnsonw <wjohnson@whamcloud.com>
Signed-off-by: johnsonw <wjohnson@whamcloud.com>
Signed-off-by: johnsonw <wjohnson@whamcloud.com>
@@ -136,7 +136,7 @@ pub(crate) struct QueryRoot;
#[juniper::graphql_object(Context = Context)]
impl QueryRoot {
#[graphql(arguments(
limit(description = "paging limit, defaults to 20"),
limit(description = "paging limit, defaults to All"),
Copy link
Member

Choose a reason for hiding this comment

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

Maybe:

Suggested change
limit(description = "paging limit, defaults to All"),
limit(description = "optional paging limit, defaults to all rows"),

Signed-off-by: johnsonw <wjohnson@whamcloud.com>
@jgrund jgrund self-requested a review September 24, 2020 16:56
@jgrund jgrund merged commit af196e3 into master Sep 24, 2020
@jgrund jgrund deleted the GH-2268 branch September 24, 2020 18:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update the limit param in graphql api's
3 participants