Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Epic: Decommission Legacy OCW Implementations / Architecture Concessions #235

Open
2 tasks
rhysyngsun opened this issue Apr 27, 2021 · 4 comments
Open
2 tasks

Comments

@rhysyngsun
Copy link
Collaborator

Summary

This issue documents parts of our implementation that were implemented as a bridge between the legacy OCW site and ocw-next.

Please make note of what these things need to be removed or cleaned up and any potential issues that could come up. More detail is better.

Code to Remove

  • Remove ocw_import app
    • This can probably be removed entirely

Architectural Cleanup

  • The triggering of WebsiteContent syncing should be moved into django signals
    • This was a design concession to support bulk import of courses without incurring large numbers of github commits. Currently this is triggered by explicit task invocations in the web API. Those should be removed and the calls moved into a signal.
@mbertrand
Copy link
Member

Should we remove the single_website_task decorator (website-specific redis lock) when running the content_sync.tasks.sync_all_content task? Should we go back to individual file commits whenever a WebsiteContent object is created/updated instead of running that task?

@rhysyngsun
Copy link
Collaborator Author

Should we remove the single_website_task decorator (website-specific redis lock) when running the content_sync.tasks.sync_all_content task? Should we go back to individual file commits whenever a WebsiteContent object is created/updated instead of running that task?

I think there's still a benefit to doing it this way - it avoids a whole category of issues/quirks that could arise from attempting multiple concurrent writes to the repos. I don't think there's a huge penalty here either because API requests in general should move along quickly.

@mbertrand
Copy link
Member

I think there's still a benefit to doing it this way - it avoids a whole category of issues/quirks that could arise from attempting multiple concurrent writes to the repos.

Sounds good to me, in that case should we remove the api/backend methods for making individual commits? GithubBackend.update_content_in_backend, GithubBackend.create_content_in_backend, etc?

@rhysyngsun
Copy link
Collaborator Author

I think there's still a benefit to doing it this way - it avoids a whole category of issues/quirks that could arise from attempting multiple concurrent writes to the repos.

Sounds good to me, in that case should we remove the api/backend methods for making individual commits? GithubBackend.update_content_in_backend, GithubBackend.create_content_in_backend, etc?

Yeah, I think we can just have methods that commit a set of changes, and if we ever need to sync 1 object in the future that's just a set w/ size=1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants