Revert "Batch queries to fetch all courses to prevent over-fetching during paging operations"
This reverts commit 544c7f7eff.
This commit is contained in:
Calen Pennington
@@ -1,38 +0,0 @@
|
||||
SELECT `course_duration_limits_coursedurationlimitconfig`.`id`,
|
||||
`course_duration_limits_coursedurationlimitconfig`.`change_date`,
|
||||
`course_duration_limits_coursedurationlimitconfig`.`changed_by_id`,
|
||||
`course_duration_limits_coursedurationlimitconfig`.`enabled`,
|
||||
`course_duration_limits_coursedurationlimitconfig`.`site_id`,
|
||||
`course_duration_limits_coursedurationlimitconfig`.`org`,
|
||||
`course_duration_limits_coursedurationlimitconfig`.`course_id`,
|
||||
`course_duration_limits_coursedurationlimitconfig`.`enabled_as_of`,
|
||||
1 AS `is_active`
|
||||
FROM `course_duration_limits_coursedurationlimitconfig`
|
||||
WHERE (
|
||||
`course_duration_limits_coursedurationlimitconfig`.`id` IN
|
||||
(
|
||||
SELECT max(u0.`id`) AS `max`
|
||||
FROM `course_duration_limits_coursedurationlimitconfig` u0
|
||||
GROUP BY u0.`site_id`,
|
||||
u0.`org`,
|
||||
u0.`course_id`
|
||||
ORDER BY NULL)
|
||||
AND ((
|
||||
`course_duration_limits_coursedurationlimitconfig`.`course_id` IS NULL
|
||||
AND `course_duration_limits_coursedurationlimitconfig`.`org` IS NULL
|
||||
AND `course_duration_limits_coursedurationlimitconfig`.`site_id` IS NULL)
|
||||
OR (
|
||||
`course_duration_limits_coursedurationlimitconfig`.`course_id` IS NULL
|
||||
AND `course_duration_limits_coursedurationlimitconfig`.`org` IS NULL
|
||||
AND `course_duration_limits_coursedurationlimitconfig`.`site_id` = 1)
|
||||
OR (
|
||||
`course_duration_limits_coursedurationlimitconfig`.`course_id` IS NULL
|
||||
AND `course_duration_limits_coursedurationlimitconfig`.`org` = 'edX'
|
||||
AND `course_duration_limits_coursedurationlimitconfig`.`site_id` IS NULL)
|
||||
OR (
|
||||
`course_duration_limits_coursedurationlimitconfig`.`course_id` = 'course-v1:UCSanDiegoX+ALGS201x+2T2017'
|
||||
AND `course_duration_limits_coursedurationlimitconfig`.`org` IS NULL
|
||||
AND `course_duration_limits_coursedurationlimitconfig`.`site_id` IS NULL)))
|
||||
ORDER BY IF(Isnull(`course_duration_limits_coursedurationlimitconfig`.`course_id`),0,1), `course_duration_limits_coursedurationlimitconfig`.`course_id` DESC,
|
||||
IF(isnull(`course_duration_limits_coursedurationlimitconfig`.`org`),0,1), `course_duration_limits_coursedurationlimitconfig`.`org` DESC,
|
||||
IF(isnull(`course_duration_limits_coursedurationlimitconfig`.`site_id`),0,1), `course_duration_limits_coursedurationlimitconfig`.`site_id` DESC
|
||||
@@ -389,8 +389,7 @@ class CourseListSearchViewTest(CourseApiTestViewMixin, ModuleStoreTestCase, Sear
|
||||
self.setup_user(self.audit_user)
|
||||
|
||||
# These query counts were found empirically
|
||||
query_counts = [64, 51, 51, 51, 51, 51, 51, 51, 51, 51, 21]
|
||||
with_username_adjust = [4, 4, 4, 7, 7, 7, 10, 10, 10, 13, 13]
|
||||
query_counts = [63, 50, 50, 50, 50, 50, 50, 50, 50, 50, 20]
|
||||
ordered_course_ids = sorted([str(cid) for cid in (course_ids + [c.id for c in self.courses])])
|
||||
|
||||
self.clear_caches()
|
||||
@@ -399,7 +398,7 @@ class CourseListSearchViewTest(CourseApiTestViewMixin, ModuleStoreTestCase, Sear
|
||||
RequestCache.clear_all_namespaces()
|
||||
expected_query_count = query_counts[page - 1]
|
||||
if with_username:
|
||||
expected_query_count += with_username_adjust[page - 1]
|
||||
expected_query_count += 4
|
||||
with self.assertNumQueries(expected_query_count):
|
||||
params = {'page': page, 'page_size': 30}
|
||||
if with_username:
|
||||
|
||||
@@ -28,7 +28,6 @@ from django.conf import settings
|
||||
from django.db.models import Prefetch, prefetch_related_objects
|
||||
from django.urls import reverse
|
||||
from django.http import Http404, QueryDict
|
||||
from django_chunked_iterator import batch_iterator, iterator
|
||||
from enrollment.api import get_course_enrollment_details
|
||||
from edxmako.shortcuts import render_to_string
|
||||
from fs.errors import ResourceNotFound
|
||||
@@ -454,21 +453,21 @@ def get_course_syllabus_section(course, section_key):
|
||||
raise KeyError("Invalid about key " + str(section_key))
|
||||
|
||||
|
||||
def get_courses(user, org=None, filter_=None, batch_size=100):
|
||||
def get_courses(user, org=None, filter_=None):
|
||||
"""
|
||||
Return a LazySequence of courses available, optionally filtered by org code (case-insensitive).
|
||||
"""
|
||||
courses_qset = branding.get_visible_courses(
|
||||
courses = branding.get_visible_courses(
|
||||
org=org,
|
||||
filter_=filter_,
|
||||
).select_related(
|
||||
'image_set'
|
||||
).prefetch_related(
|
||||
Prefetch(
|
||||
'modes',
|
||||
queryset=CourseMode.objects.exclude(mode_slug__in=CourseMode.CREDIT_MODES),
|
||||
to_attr='selectable_modes',
|
||||
),
|
||||
).select_related(
|
||||
'image_set'
|
||||
)
|
||||
|
||||
permission_name = configuration_helpers.get_value(
|
||||
@@ -476,30 +475,10 @@ def get_courses(user, org=None, filter_=None, batch_size=100):
|
||||
settings.COURSE_CATALOG_VISIBILITY_PERMISSION
|
||||
)
|
||||
if user.is_authenticated:
|
||||
prefetch_related_objects([user], 'roles', 'experimentdata_set')
|
||||
|
||||
def with_prefetched_users():
|
||||
for courses in batch_iterator(courses_qset, batch_size=batch_size):
|
||||
prefetch_related_objects(
|
||||
[user],
|
||||
Prefetch(
|
||||
'courseenrollment_set',
|
||||
queryset=CourseEnrollment.objects.filter(
|
||||
user=user,
|
||||
course__in=courses
|
||||
).select_related('schedule')
|
||||
)
|
||||
)
|
||||
for course in courses:
|
||||
if has_access(user, permission_name, course):
|
||||
yield course
|
||||
|
||||
course_iterator = with_prefetched_users()
|
||||
else:
|
||||
course_iterator = (c for c in iterator(courses_qset) if has_access(user, permission_name, c))
|
||||
prefetch_related_objects([user], 'roles', 'courseenrollment_set', 'experimentdata_set')
|
||||
return LazySequence(
|
||||
course_iterator,
|
||||
est_len=courses_qset.count()
|
||||
(c for c in courses if has_access(user, permission_name, c)),
|
||||
est_len=courses.count()
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -37,7 +37,6 @@ celery==3.1.25 # Asynchronous task execution library
|
||||
defusedxml
|
||||
Django==1.11.20 # Web application framework
|
||||
django-babel-underscore # underscore template extractor for django-babel (internationalization utilities)
|
||||
django-chunked-iterator # Allow more control over how large queries are executed
|
||||
django-config-models>=0.2.2 # Configuration models for Django allowing config management with auditing
|
||||
django-cors-headers==2.1.0 # Used to allow to configure CORS headers for cross-domain requests
|
||||
django-countries==4.6.1 # Country data for Django forms and model fields
|
||||
|
||||
@@ -68,7 +68,6 @@ defusedxml==0.5.0
|
||||
django-appconf==1.0.3 # via django-statici18n
|
||||
django-babel-underscore==0.5.2
|
||||
django-babel==0.6.2 # via django-babel-underscore
|
||||
django-chunked-iterator==0.6.1
|
||||
django-classy-tags==0.8.0 # via django-sekizai
|
||||
django-config-models==0.2.2
|
||||
django-cors-headers==2.1.0
|
||||
|
||||
@@ -89,7 +89,6 @@ distlib==0.2.8
|
||||
django-appconf==1.0.3
|
||||
django-babel-underscore==0.5.2
|
||||
django-babel==0.6.2
|
||||
django-chunked-iterator==0.6.1
|
||||
django-classy-tags==0.8.0
|
||||
django-config-models==0.2.2
|
||||
django-cors-headers==2.1.0
|
||||
|
||||
@@ -87,7 +87,6 @@ distlib==0.2.8 # via caniusepython3
|
||||
django-appconf==1.0.3
|
||||
django-babel-underscore==0.5.2
|
||||
django-babel==0.6.2
|
||||
django-chunked-iterator==0.6.1
|
||||
django-classy-tags==0.8.0
|
||||
django-config-models==0.2.2
|
||||
django-cors-headers==2.1.0
|
||||
|
||||
Reference in New Issue
Block a user