Update logic to account for /versions/ and /thumbnails/ endpoints que… #80
Conversation
jashan-lco
left a comment
jashan-lco
left a comment
There was a problem hiding this comment.
Makes sense, but I really wish DRF provided a easy way to set pagination classes per view to separate things out a bit
| self.small_query = True | ||
| elif 'request_id' in query_params or 'observation_id' in query_params: | ||
| # /versions/?md5= queries need the accurate count for shipping data in | ||
| elif request.path == '/versions/' and 'md5' in query_params: |
There was a problem hiding this comment.
Stupid Django question...but is the path normalized? Like if they go to /versions?limit=1 does that always redirect to /versions/?limit=1
There was a problem hiding this comment.
Yeah it seems to get http 301'ed to the /versions/?limit=1 if you do the first
| self.small_query = True | ||
| # Or up to 2 months of querys with some other bounding params | ||
| elif timespan <= timedelta(weeks=9) and any(field in query_params for field in ['proposal_id', 'target_name_exact']): | ||
| self.small_query = True |
There was a problem hiding this comment.
imo, an explicit terminal else clause makes it easier for my brain to process if/else branches...especially nested ones
It does provide an easy way to separate pagination class per view - you can just set it on the view itself and it overrides the default setting in settings.py. Do you think that would be better? I figured there would be mostly overlap besides that if/else block but I'm happy to pull it out into separate pagination classes if you think that is better. |
Oh, really? Couldn't figure that out from the docs. IMO it might make things a bit more maintainable going forward and you'll get rid of the outer if/else branches. |
I fully implemented it as separate pagination classes, but then realized the viewset mixer allowing limitoffset vs. cursor would be broken if we wanted to use it on the other endpoints later, which we might. So I think I vote to just keep it as-is for now. |
…ries too
This addresses the bug in /versions/ queries, but also adds in exact counts for /thumbnails/ queries if any of its indexed fields are used.