bakert
commented
5 months ago Also while we're in there we should decide if these funcs return a Sequence or a list. They are currently split between the two. Sequence does allow the changing of implementation in future but that doesn't seem important compared to the practical benefit of not having to cast the list to a list in order to use it as a list in a type-safe way as we do in at least one place (list(matchup_people)).
And we should probably move the removing of "total" from result objects into a helper func as we do it in so many places. [Container({k: v for k, v in r.items() if k != 'total'}) for r in rs] … could form these from rs even and return the total too.
I refactored them to return {results, total} because that's what the API wants. And because in general it's dangerous to form queries that don't have a limit. The refactoring made it so that we're issuing one query to get results+total instead of two queries. However in practical terms almost all the calls to these funcs don't include a limit and discard the total as irrelevant. So perhaps load_decks and friends should go back to having a single return value, and you call load_decks_with_total or something from the API. Or perhaps every callsite should be examined for potential perf issues. I dropped slow_query to 500ms yesterday and haven't heard anything so perhaps eveyrhting we're doing is just fine. Needs some thought.