Moodle

gradebook doesn't throw any events

Details

  • Type: Bug Bug
  • Status: Open Open
  • Priority: Minor Minor
  • Resolution: Unresolved
  • Affects Version/s: 1.9.5
  • Fix Version/s: DEV backlog
  • Component/s: Events API, Gradebook
  • Labels:
    None
  • Affected Branches:
    MOODLE_19_STABLE

Description

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

Activity

Hide
Petr Škoda (skodak) added a comment -

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?

Show
Petr Škoda (skodak) added a comment - 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?
Hide
Penny Leach added a comment -

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.

Show
Penny Leach added a comment - 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.
Hide
Penny Leach added a comment -

Also regarding performance, any event listeners that are going to do anything large should schedule their pickup for cron, not during the apache request.

Show
Penny Leach added a comment - Also regarding performance, any event listeners that are going to do anything large should schedule their pickup for cron, not during the apache request.
Hide
Petr Škoda (skodak) added a comment -

yes, cron processing means only 2x queries

Show
Petr Škoda (skodak) added a comment - yes, cron processing means only 2x queries
Hide
Penny Leach added a comment -

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.

Show
Penny Leach added a comment - 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.
Hide
Petr Škoda (skodak) added a comment -

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.

Show
Petr Škoda (skodak) added a comment - 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.
Hide
Petr Škoda (skodak) added a comment - - 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...

Show
Petr Škoda (skodak) added a comment - - 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...
Hide
Matt Gibson added a comment -

What's the current thinking on this? I could really use it for a number of things in some 2.0 plugins I'm working on, so I'm happy to do some work and make a patch if there is any consensus.

Perhaps the grade item updated event could be one event for a single recalculation, but containing a large data object with all the changed grades in it, which the handlers would just have to deal with.

Show
Matt Gibson added a comment - What's the current thinking on this? I could really use it for a number of things in some 2.0 plugins I'm working on, so I'm happy to do some work and make a patch if there is any consensus. Perhaps the grade item updated event could be one event for a single recalculation, but containing a large data object with all the changed grades in it, which the handlers would just have to deal with.

People

Vote (1)
Watch (1)

Dates

  • Created:
    Updated: