Skip to content

Conversation

@rubendura
Copy link
Contributor

Refs #3329

Solves a memory issue when evaluating very large querysets

@lovelydinosaur
Copy link
Contributor

As presented I think this would actually break the html_cutoff displays, by not including the 'Over 1000 items' text. I'm also not sure how keen I am on introducing the cutoff to .choices itself.

An alternative might be a get_choices(cutoff=...) method. The .choices property could call it directly without a cutoff, and iter_options could call it, with the cutoff+1?...

@rubendura
Copy link
Contributor Author

It seems to be working fine on my side with a dropdown using html_cutoff, but I haven't tested other cases yet.

I'm not sure what you mean with the last paragraph. Why would you want to call that get_choices() with different cutoffs?

@lovelydinosaur
Copy link
Contributor

What I mean is I'm not totally comfortable with '.choices' always being hard-limited.
We ought to ensure that .html_cutoff really only does apply when rendering to html, rather than affecting any other usages.

(Tho its possible I could be convinced to change my mind on that)

@rubendura
Copy link
Contributor Author

I guess that by doing this we keep the current RelatedField.choices behaviour while allowing iter_options to limit how many choices we have. I know this is lacking tests and we should probably have some for it. I might look into it later.

@lovelydinosaur
Copy link
Contributor

Looks about right, yeah.

@vstoykov
Copy link
Contributor

vstoykov commented Sep 7, 2015

I also have problems with RelatedField to table with many records. OPTIONS response can be delay to 1 minute which is not good. I also thought about proper solution for this problem and I don't think that limiting .choices is a good idea.

  1. First I'm not sure what will happen with validation
  2. If we can't get all of the choices probably we need to create another API endpoint which will return filtered results based on request (for autocomplete for example). Then if we have such an endpoint why we need choices for that field at all?

Probably we need an argument for that field that will mark the field as "raw_id" like in the django admin. And for this field there will be no select in the HTML view and there will be no choices in the OPTIONS response.

What you think about that?

@lovelydinosaur
Copy link
Contributor

Probably we need an argument for that field that will mark the field as "raw_id" like in the django admin. And for this field there will be no select in the HTML view and there will be no choices in the OPTIONS response.

I think what we present in OPTIONS responses is a diff issue to what we present in HTML selections. Personally, right now I'd rather just we weren't showing them in OPTIONS at all.

@jathanism
Copy link

I would like to add that I am encountering this issue in my own server (with about 220k related records for a field) which utterly crushes the app.

I tested the patch above and it's working for me.

You also have my vote for removing the enumeration of related choices in OPTIONS!

@lovelydinosaur lovelydinosaur added this to the 3.3.3 Release milestone Jan 27, 2016
@lovelydinosaur
Copy link
Contributor

You also have my vote for removing the enumeration of related choices in OPTIONS!

Probably a good plan.

@xordoquy xordoquy modified the milestones: 3.3.3 Release, 3.3.4 Release, 3.4.0 Release Feb 11, 2016
@wimglenn
Copy link
Contributor

+1 choices enumeration in an OPTIONS request is making the response HUGE, and the browsable API unusable for me in 3.3.2

@wimglenn
Copy link
Contributor

Thanks! 👍

@lovelydinosaur
Copy link
Contributor

lovelydinosaur commented Aug 10, 2016

Most welcome. Pleasure to get the triage back on track now that we're (mostly) funded! 😎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants