fix: delay update_outline_from_modulestore_task after course publish.

[Jacatove]

Description

Once a course is created in studio the course_published signal is emitted, therefore some receivers trigger some async tasks which end up failing because the modulestore can not find the latest data, which resulted in errors related to not being able to find the course, Check this discussion for more information and how to replicate those errors. However, there is another issue when working with CCXs, this same error raises from time to time when one creates a CCX, therefore the CCX's outline is not generated and the course content is not showed. Currently it requires that someone manually regenerates the outline at http://localhost:18000/admin/contentstore/courseoutlineregenerate/

Changes

• Add a delay for update_outline_from_modulestore_task

Supporting information

The change proposed in this PR is based on: #31307

How to replicate the issue?

• Try creating multiple CCXs some of them might raise an error when loading the course, since the update_outline_from_modulestore_task should have failed when course was published.
• Once you create a course in studio, the update_outline_from_modulestore_task will fail, I understand that this is not an issue as the course is empty and adding content would just recreate the outline as well as other tasks.

Testing instructions

• Create a course, the outcome should be the same as before.
• Create a CCX, the outline should be generated.

[openedx-webhooks]

Thanks for the pull request, @Jacatove!

What's next?

Please work through the following steps to get your changes ready for engineering review:

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

• If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
  • This process (including the steps you'll need to take) is documented here.
• If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

• Dependencies

  This PR must be merged before / after / at the same time as ...

• Blockers

  This PR is waiting for OEP-1234 to be accepted.

• Timeline information

  This PR must be merged by XX date because ...

• Partner information

  This is for a course on edx.org.

• Supporting documentation
• Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

🔘 Let us know that your PR is ready for review:

Who will review my changes?

This repository is currently maintained by @openedx/wg-maintenance-edx-platform. Tag them in a comment and let them know that your changes are ready for review.

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

• Overview of Review Process for Community Contributions
• Pull Request Status Guide
• Making changes to your pull request

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

• The size and impact of the changes that it introduces
• The need for product review
• Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

[navinkarkera]

@mphilbrick211 @Jacatove I can help review this PR as CC.

[mphilbrick211]

@mphilbrick211 @Jacatove I can help review this PR as CC.

Thank you, @navinkarkera - though it looks like we're still missing a CLA form for @Jacatove. @Jacatove could you please confirm if you previously submitted a CLA form?

[Jacatove]

@mphilbrick211 Yes, I have completed that form.

[mphilbrick211]

@mphilbrick211 Yes, I have completed that form.

Hi @Jacatove! I don't think we have it on file any longer. Do you remember when you first submitted it? It might help to re-submit if you're able.

[Jacatove]

@mphilbrick211 I resubmitted the form some days ago.

[mariajgrimaldi]

I'll leave this here in case it helps in some way:
I tried doing this before to solve another issue. Please see the discussion here: #34020. I squashed the commits when merging, so you can't see my first proposal that started the conversation. But it was something like this:

    if key_supports_outlines(course_key):
        # Push the course outline to learning_sequences asynchronously.
        update_outline_from_modulestore_task.apply_async(
            args=[course_key_str],
            countdown=settings.BLOCK_STRUCTURES_SETTINGS['COURSE_PUBLISH_TASK_DELAY'],
        ) 

Specifically, this part where Dave says:

The reason I ask is that "delay be X seconds" inevitably causes operational race conditions at some point (e.g. when a piece of middleware starts hanging because it's contacting some service that has failed). If we can make sure the change is committed and visible before the celery queuing task executes, that would be ideal.

[mphilbrick211]

@mphilbrick211 I resubmitted the form some days ago.

Hi @Jacatove! I checked on this for you. Our legal staff was out of the office, but she returns tomorrow, and your CLA should be processed by the end of the week. Thank you for your patience!

[mphilbrick211]

@mphilbrick211 I resubmitted the form some days ago.

Hi @Jacatove! I checked on this for you. Our legal staff was out of the office, but she returns tomorrow, and your CLA should be processed by the end of the week. Thank you for your patience!

Hi @Jacatove! I see that your CLA form has gone through, and the CLA check should now turn green. Do you plan to pursue this PR?

[navinkarkera]

@Jacatove The code changes look good based on other similar implemenations as I could not really test it due CCX tables being missing in my local setup.

@mariajgrimaldi What do you think? Do we need to involve someone like Dave before merging this?

[mphilbrick211]

@Jacatove The code changes look good based on other similar implemenations as I could not really test it due CCX tables being missing in my local setup.

@mariajgrimaldi What do you think? Do we need to involve someone like Dave before merging this?

Friendly ping on this, @mariajgrimaldi!

[mariajgrimaldi]

Thanks for the patience!

@navinkarkera: As Dave mentioned in my previous PR, we need to ensure we're not introducing race conditions with this change. Here's the comment where Dave suggested taking a different approach: #34020 (comment). I think we should first try this other approach before committing to this change: If we can make sure the change is committed and visible before the celery queuing task executes, that would be ideal

I'll tag @ormsbee for visibility.

[navinkarkera]

@mariajgrimaldi Thanks! From what I understand, we need to do the same with the views that are triggering the publish signal i.e. set transaction.non_atomic_requests to ccx views like create_ccx, save_ccx and set_grading_policy. This way the new ccx data will already be in the database before the request completes and the celery tasks that starts before request ends should have access to the latest data.

@Jacatove Can you kindly try this out and post your findings here?

[alexjmpb]

Hi @navinkarkera I'm part of the team @Jacatove belonged to, I will take care of this PR now. I will try this

set transaction.non_atomic_requests to ccx views like create_ccx, save_ccx and set_grading_policy. This way the new ccx data will already be in the database before the request completes and the celery tasks that starts before request ends should have access to the latest data.

And I'll let you know the details. Thanks!!

[mphilbrick211]

Hi @navinkarkera I'm part of the team @Jacatove belonged to, I will take care of this PR now. I will try this

set transaction.non_atomic_requests to ccx views like create_ccx, save_ccx and set_grading_policy. This way the new ccx data will already be in the database before the request completes and the celery tasks that starts before request ends should have access to the latest data.

And I'll let you know the details. Thanks!!

Hi @alexjmpb just checking to see if this is still in progress?