Issue Details (XML | Word | Printable)

Key: MDL-19261
Type: Bug Bug
Status: Open Open
Priority: Minor Minor
Assignee: Petr Skoda
Reporter: Penny Leach
Votes: 0
Watchers: 1
Operations

Add/Edit UI Mockup to this issue
If you were logged in you would be able to see more operations.
Moodle

gradebook doesn't throw any events

Created: 20/May/09 08:05 PM   Updated: 28/Sep/09 02:32 AM
Return to search
Component/s: Events API, Gradebook
Affects Version/s: 1.9.5
Fix Version/s: 2.0

Participants: Penny Leach and Petr Skoda
Security Level: None
Affected Branches: MOODLE_19_STABLE
Fixed Branches: MOODLE_20_STABLE


 Description  « Hide
could grade_update please throw a couple of events?

like

grade_added
grade_updated
grade_deleted


I am kind of confused by this because I thought the event API was written *for* the gradebook ;)

 All   Comments   Change History   Version Control      Sort Order: Ascending order - Click to sort in descending order
Petr Skoda added a comment - 20/May/09 08:35 PM
Well, it was but then we realised that the performance would be really horrible which was not an option
You can use automated grade exports for now if you need grades in external systems.
There is also the problem with gradeable roles, we do not really know if user is expected to have any grades in course when recalculating grades.

Another problem is that final grade values are calculated only when you actually need them, which means the external system would not be notified immediately.

I am afraid that triggering events separately for each grade might multiply the number of sql queries thus making grader report operations even slower.

I see you marked fix version 1.9.6, why? Are you going to work on this? Did anybody promise this feature to you?


Penny Leach added a comment - 20/May/09 10:16 PM
Because I talked to Nico about it. and he said he'd look at it.

I guess I'll just go ahead and make another customisation. If you are unwilling to support people making local customisations in 1.9 please at least fix it in 2.0. Moodle used to be really good at having core open to hooks for people doing slightly special things.


Penny Leach added a comment - 20/May/09 10:23 PM
Also regarding performance, any event listeners that are going to do anything large should schedule their pickup for cron, not during the apache request.

Petr Skoda added a comment - 20/May/09 10:38 PM
yes, cron processing means only 2x queries

Penny Leach added a comment - 20/May/09 10:53 PM
I'm kind of baffled by that. You either mean 2 queries, or 2 TIMES the queries.

I have to say, I'm still kind of of the opinion that

  • The event TYPE should be responsible for knowing whether it should be processed at cron or immediately, not the handlers (or at least can force cron handling)
  • calling events_trigger should 95% of cases do nothing more than insert_record('event', (object)array($eventtype, serialise($eventdata));

Events should be thrown indiscriminately and caught conservatively, just like exceptions.


Petr Skoda added a comment - 20/May/09 10:56 PM
Well, Moodle is also notoriously known for unfinished/unmaintained parts of code and regressions in stable branch, sorry I am just trying to prevent this. I would like to work on new hooks and feature, but STABLE branch is clearly not a good place for this. Also Martin wants all core developers to concentrate on 2.0 and I agree with that. I personally think that enrolment events are more important than gradebook events, but we could not implement that for obvious performance problems with current roles design - I already proposed a solution and tested implementation, that is why I am oposing any new features with perf problems that would be fixed in my design.

I understand that partners and admins work mostly with stable branch and development in HEAD is not interesting for them and that is why the submit feature requests for already stable branches. If we want to make Moodle more reliable and stable should do this in HEAD only. This is a perfect example, this should have been already done in 1.9dev, not 1.9.6 - this feature may seem trivial, but in the end it might require weeks for implementation+testing, on the other hand if we do it in HEAD only it will be a few days of work and the testing would be done together with other changes later, there would be also 0% risk of regressions in stable branch. Once it lands in HEAD it would be much easier for you to backport it to 1.9x for your customers without the risk of unexpected changes in HEAD.


Petr Skoda added a comment - 20/May/09 11:04 PM - edited
When recalculating grades the biggest number of calls are updates because they are done separately for each grade, the gradebook structure and grades are loaded with fixed number of queries. So the triggering of events costs 1 query for fetching the list of handlers (cached) plus the same number of grade updates. Then in the handler you must process the thousands of changes of grades from different courses, so either you have to write extremely clever code that somehow somewhere caches the grades from event loop or process them one by one, at the same time you have to make sure you do not run out of memory. Now compare this to simple periodic pulling of grades from gradebook using export plugin...