Check that email subtasks are known to the InstructorTask before executing. #1351
Conversation
|
👍 |
There was a problem hiding this comment.
Maybe add a comment of why could the check fail, so a developer can figure out what to do or check if that is the case.
|
I had one minor comment. Once it is addressed it is good to merge 👍 |
|
I found in a log that the same task (i.e. submit_bulk_email) had been submitted twice by celery, due to some loss of connection or something. I'll look closer at the traceback later, but in the meantime I can at least check for the condition, and avoid the duplication. If I don't catch it at the parent task level, I should then be able to catch it at the subtask level. @sarina @rocha another look, please? |
There was a problem hiding this comment.
I'm not 100% clear why we know it's the exact same task that's called and not another, new task that is spawned. I feel I'm missing some puzzle piece...
There was a problem hiding this comment.
Yeah I agree, it probably requires further investigation. However, if this has to be release soon I am ok with leaving it as it is provided we come back to figure out the problem later. Seems like something worth fixing going down the road.
There was a problem hiding this comment.
The task is defined by the InstructorTask object. It contains the task_type, the task_id, and the input values. If that InstructorTask object arrives at this point in the code, and we find that it already contains subtasks, then we know we don't need to define subtasks for it. (I check the output because I need it to exist before returning successfully.) And there's really no other place where the subtasks would come from, and even if they did (because something was horribly wrong), it wouldn't make sense to write over them anyway.
There was a problem hiding this comment.
Ah that makes sense. thanks.
|
👍 but please do look later at the pending issues |
|
👍 |
Check that email subtasks are known to the InstructorTask before executing.
* Revert "version bump for scorm xblock (openedx#1354)" This reverts commit d465dc3. * Revert "Bump completion to v0.1.15 (openedx#1351)" This reverts commit 58932f3.
Addresses a problem where bulk email subtasks are duplicated at the point that a group of subtasks is queued. Those with a requested subtask_id are created correctly, and duplicate subtasks with different ids are also created with otherwise identical arguments. This fix prevents the duplicate subtasks from actually running (and sending duplicate emails).