Panopto / Moodle-2.0-plugin-for-Panopto

Panopto's integration with the Moodle LMS.
http://www.panopto.com
GNU General Public License v3.0
18 stars 38 forks source link

Move rolling sync to adhoc task #33

Closed sk-unikent closed 9 years ago

sk-unikent commented 9 years ago

Having the SOAP calls in the observers means that role/enrolment changes are much slower. We sync nightly with our student data system, and that can easily pull in 100's or 1000's of enrolment/role changes, this really slows that down.

This makes the observers spawn an adhoc task to deal with the SOAP calls later in cron, which takes the slowdown caused by the api queries away from end-users.

Panopto commented 9 years ago

Hello!

We did a review of this pull request and we have determined that this is a good idea if we can put it behind a configuration switch. We don't want to require everyone who uses this block to have asynchronous tasks passing the enrollment update information.

If you have the time to put in this switch to make the behavior conditional, we would very much appreciate that. If not, it may be some time before we can add this change.

Thanks, Jenn

sk-unikent commented 9 years ago

I've added a switch and made it early-exit when the course doesn't have a Panopto mapping. The default is to use the current method - inline sync.

Panopto commented 9 years ago

This is looking good, and everything worked perfectly in testing. Thank you! We will merge now.

kabalin commented 9 years ago

Notice that https://github.com/Panopto/Moodle-2.0-Plugin-for-Panopto/blob/master/classes/task/update_user.php has Moodle GPL copyright statement at the top, while the rest of files are "Panopto plugin" licensed.

@Panopto you guys should consider sticking to one or the other (preferably using Moodle license as most of Moodle contrib plugins do) ;) There is also a file that is licensed twice :-/ (ignore for now, I fixed that in the pull request I am preparing at the moment).

Panopto commented 9 years ago

Thanks for the comment about cleaning up the licensing message. I filed that as an internal work item.