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

Unable to use "distinct" parameter #10

Closed
dccarroll opened this issue Apr 17, 2014 · 4 comments
Closed

Unable to use "distinct" parameter #10

dccarroll opened this issue Apr 17, 2014 · 4 comments

Comments

@dccarroll
Copy link
Contributor

The command: clever.Section.all(distinct="period") leads to the error: InvalidRequestError: Can only use "distinct" in conjuction with "where" and "limit".

But the command: periods = clever.Section.all(distinct="period",limit=100) also leads to an error: CleverError: ListableAPIResource does not support 'limit' parameter

The limit parameter should be supported in conjunction with the "distinct" parameter.

@azylman
Copy link

azylman commented Apr 17, 2014

The first error is happening because we add page parameters to all requests from all. Because we paginate automatically, we disallow sending limit and page, which causes the second error.

Not sure if there should be a special case for distinct in these functions, or if distinct should be broken out into its own method, a.la. clever.Section.distinct("period").

I think from a readability/code complexity point of view, the second one is probably better (and I would lean towards that), but you could also make an argument that the smaller the API profile, the better (e.g. currently we only have two methods: iter and all).

@nathanleiby
Copy link
Contributor

@azylman why doesn't distinct allow pagination?

@nathanleiby
Copy link
Contributor

@dccarroll - Interesting catch - I didn't realize these were interdependent when we hardcoded limit for pagination purposes.

limit implies two things (1) MAX results to return (2) PAGE_SIZE. I suggest that clever-python hides (2) from the library consumer. (1) seems a much more sensible way of thinking about it from the library's perspective.

I don't like adding another method either; however, we might consider that the parameters need not exactly map to the API. as we do with all(), there could be more abstraction between the goal (fetching distinct items) and exactly what query parameters get passed behind the scenes to the API.

@azylman
Copy link

azylman commented Apr 17, 2014

No idea, I just know the API rejects any query parameters with distinct except where and limit.

Maybe the aggregation pipeline (which we use for distinct) doesn't offsets?

@nathanleiby nathanleiby reopened this Apr 17, 2014
@mcab mcab closed this as not planned Won't fix, can't repro, duplicate, stale Jun 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants