Skip to content

fix: fix race condition where library is not in the store yet#826

Open
marslanabdulrauf wants to merge 4 commits into
mainfrom
marslan/12267-git-auto-export-missing-repo
Open

fix: fix race condition where library is not in the store yet#826
marslanabdulrauf wants to merge 4 commits into
mainfrom
marslan/12267-git-auto-export-missing-repo

Conversation

@marslanabdulrauf

Copy link
Copy Markdown
Contributor

What are the relevant tickets?

https://github.com/mitodl/hq/issues/12267

Description (What does it do?)

This pull request improves the reliability of exporting courses and libraries to GitHub by introducing better error handling for cases where content is not yet available in the database due to transaction timing. It adds a custom exception, ensures Celery tasks are dispatched only after database commits, and updates error handling in utility and task functions.

How can this be tested?

  1. Make sure it setup properly and configurations are correct
  2. Creating a new library should always create new repository
  3. Library repository's readme should have correct studio url

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves the robustness of the ol_openedx_git_auto_export plugin when exporting newly-created courses/libraries to GitHub by handling “content not found (yet)” scenarios and deferring certain Celery dispatches until after DB commit, reducing transaction-timing races.

Changes:

  • Introduces ContentNotFoundError and updates get_content_info to raise it when courses/libraries can’t be found (including handling ContentLibraryNotFound from the v2 libraries API).
  • Updates Celery tasks to treat missing content as a non-fatal/expected condition (logging and skipping/returning cleanly).
  • Defers the library v2 “create repo + export” task dispatch using transaction.on_commit(...) to avoid running before the creating transaction commits.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/utils.py Raises a plugin-specific not-found exception for v1/v2 library and course lookups.
src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/tasks.py Handles ContentNotFoundError in async tasks; adjusts Studio URL path composition for repo description.
src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/signals.py Uses transaction.on_commit for v2 library creation task dispatch to prevent pre-commit races.
src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/exceptions.py Adds the new ContentNotFoundError exception type.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/tasks.py
@arslanashraf7

Copy link
Copy Markdown
Contributor

@marslanabdulrauf is this ready for review? Could you please add the Needs review label to the PR if so?

@arslanashraf7 arslanashraf7 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a small comment 👍

Comment thread src/ol_openedx_git_auto_export/ol_openedx_git_auto_export/tasks.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants