Create task to process a batch of grade computations. - #14752
Conversation
89c65dd to
66178f1
Compare
There was a problem hiding this comment.
Question for @edx/educator-neem devs: It looks like PersistentCourseGrade objects aren't getting created. Is there still a toggle for that which needs to get turned on for the test?
There was a problem hiding this comment.
I was going to suggest the write-if-engaged work might be at fault here, but that hasn't merged yet. I'd suggest manually stepping through the code in this test.
There was a problem hiding this comment.
It's working fine now. I think I must have changed something since I updated the test. Not sure what it was.
There was a problem hiding this comment.
I reintroduced testing the PersistentCourseGrade count.
66178f1 to
c624396
Compare
efischer19
left a comment
efischer19
left a comment
There was a problem hiding this comment.
LGTM, assuming the question you asked is resolved.
It seems like this and https://github.com/edx/edx-platform/pull/14771 will generate some merge conflicts depending on which lands first, heads up @nasthagiri
| @task(base=_BaseTask) | ||
| def compute_grades_for_course(course_key, offset, batch_size): # pylint: disable=unused-argument | ||
| """ | ||
| TODO: TNL-6690: Fill this task in and remove pylint disables |
There was a problem hiding this comment.
pylint disable can be removed from task signature now
There was a problem hiding this comment.
Also, update this docstring por favor
There was a problem hiding this comment.
I was going to suggest the write-if-engaged work might be at fault here, but that hasn't merged yet. I'd suggest manually stepping through the code in this test.
a91c4ae to
a3154b9
Compare
|
🚢 after jenkins is happy |
| @task | ||
| def compute_grades_for_course(course_key, offset, batch_size): # pylint: disable=unused-argument | ||
| @task(base=_BaseTask) | ||
| def compute_grades_for_course(course_key, offset, batch_size): |
There was a problem hiding this comment.
Can you update this function signature to take kwargs instead? Passing kwargs should now be the best practice for all tasks we create in the future. Otherwise, we always run into backward compatibility issues as @efischer19 and I have encountered. (I was thinking of adding a parameter to this task, related to my PR, but ran into this issue.)
Also, I believe the task isn't retried unless it's encased in a try-except block with a call to self.retry. Please consult @efischer19 on this tomorrow.
|
EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production on Thursday, March 30, 2017. |
|
|
||
| @ddt.data(*xrange(1, 12, 3)) | ||
| def test_database_calls(self, batch_size): | ||
| per_user_queries = 16 * min(batch_size, 6) # No more than 6 due to offset |
There was a problem hiding this comment.
"per_user_queries" is not referenced. I think you meant to reference it on line 413, so I'll make that change in #14731 because I need to up the number of query calls there.
|
EdX Release Notice: This PR has been deployed to the production environment. |
5 participants
TNL-6690
Create a task to perform computation of grades across a batch of users for a given course.
This leverages CourseGradeFactory.iter(), but adds a "read_only" flag, which we disable for this particular use.
Reviewers
FYI @edx/educator-neem
FYI @jibsheet -- This is the task that will be run by the management command @yro is adding to jenkins.