Repository: getsentry/sentry
Pull request: fix(workflow): Actually use 1000 limit in bulk query action
Ready to merge, fixes https://github.com/getsentry/sentry/issues/6099

Problem statement:
Bulk merge doesn't apply to all selected
Steps to reproduce
- Use the "Select the first 1,000 issues that match this search query" feature to select multiple pages of issues
- Use the UI to bulk merge issues

Upon refresh, some of the issues appear to have been merged, but many are not merged.

I did wonder if this was because the work happens asynchronously, however some issues appear to be merged immediately, leaving the user to believe the work has already completed.

Hints from issue discussion:
I feel like I noticed this too, which is odd as I thought merge changed the state synchronously. 
it's this https://github.com/getsentry/sentry/blob/master/src/sentry/api/paginator.py#L23
looked into this with jess and dena, the problem is the query is being truncated by the paginator 
it's shared across all of the bulk query actions, and the problem is in the query search. the paginator overrides the limit with its own max limit, but the endpoint doesn't actually try to paginate through the results
limit is set to 1000 here: https://github.com/getsentry/sentry/blob/master/src/sentry/api/endpoints/project_group_index.py#L451

and propagated through `search.query(**query_kwargs)` into here: 
https://github.com/getsentry/sentry/blob/master/src/sentry/search/django/backend.py#L302

the problem is that `paginator.get_result(limit, cursor, count_hits=count_hits)` overwrites the limit with a max_limit here with 100:
https://github.com/getsentry/sentry/blob/master/src/sentry/api/paginator.py#L101

and then back in `project_group_index` the paginator is cast to a list, which appears to not try to actually paginate `group_list = list(cursor_result)`
https://github.com/getsentry/sentry/blob/master/src/sentry/api/endpoints/project_group_index.py#L455

and group_list, with 100 items, is what is used for all of the bulk actions in the rest of the file, including merge and resolve.
ted mentioned, broken since: https://github.com/getsentry/sentry/commit/f1bb4a1766ea782847813c4c562b5a7ca77b369f

Commit message: pass max_limit through

Patch to review:
diff --git a/src/sentry/api/endpoints/project_group_index.py b/src/sentry/api/endpoints/project_group_index.py
index 38a43ff0625478..951d10398ed3ed 100644
--- a/src/sentry/api/endpoints/project_group_index.py
+++ b/src/sentry/api/endpoints/project_group_index.py
@@ -450,7 +450,9 @@ def put(self, request, project):
             # a bit too complicated right now
             query_kwargs['limit'] = 1000
 
-            cursor_result = search.query(**query_kwargs)
+            # the paginator has a default max_limit of 100, which must be overwritten.
+            cursor_result = search.query(
+                paginator_options={'max_limit': 1000}, **query_kwargs)
 
             group_list = list(cursor_result)
             group_ids = [g.id for g in group_list]
diff --git a/src/sentry/search/django/backend.py b/src/sentry/search/django/backend.py
index 8b88dfb91c8561..1996c38af44da7 100644
--- a/src/sentry/search/django/backend.py
+++ b/src/sentry/search/django/backend.py
@@ -273,7 +273,7 @@ def _build_queryset(
         )
         return queryset
 
-    def query(self, project, count_hits=False, **kwargs):
+    def query(self, project, count_hits=False, paginator_options={}, **kwargs):
         queryset = self._build_queryset(project=project, **kwargs)
 
         sort_by = kwargs.get('sort_by', 'date')
@@ -298,5 +298,5 @@ def query(self, project, count_hits=False, **kwargs):
             sort_clause = '-sort_value'
 
         queryset = queryset.order_by(sort_clause)
-        paginator = paginator_cls(queryset, sort_clause)
+        paginator = paginator_cls(queryset, sort_clause, **paginator_options)
         return paginator.get_result(limit, cursor, count_hits=count_hits)


Code review comments:
diff_hunk: @@ -273,7 +273,7 @@ def _build_queryset(
         )
         return queryset
 
-    def query(self, project, count_hits=False, **kwargs):
+    def query(self, project, count_hits=False, paginator_options={}, **kwargs):
path: src/sentry/search/django/backend.py
text: @user1:
Don't get into the bad habit of using mutable function arguments: http://docs.python-guide.org/en/latest/writing/gotchas/#mutable-default-arguments

@author:
wow i had no idea ted

@author:
good lord

@user1:
🙃

diff_hunk: @@ -450,7 +450,9 @@ def put(self, request, project):
             # a bit too complicated right now
             query_kwargs['limit'] = 1000
path: src/sentry/api/endpoints/project_group_index.py
text: @user1:
Maybe assign this to a local variable `limit` (e.g. `limit = query_kwargs['limit'] = 1000`) and use it below too?