UCL-INGI / INGInious

INGInious is a secure and automated exercises assessment platform using your own tests, also providing a pluggable interface with your existing LMS.
http://www.inginious.org
Other
202 stars 139 forks source link

[task_dispensers] Task settings migration #964

Closed anthonygego closed 9 months ago

anthonygego commented 1 year ago

This PR ends the long awaited process of task settings migration to task dispensers, first step to ease task sharing among courses.

As discussed with @nrybowski, the online but manual way was preferred to trigger the task file update. Therefore, a migration option is added in top of the task list page in case the current task dispenser detects legacy settings in the task files. When the upgrade is asked, the current task dispenser will import the task settings into its own data structure and then the fields imported will be removed from task file descriptors.

This is implemented by adding two function definitions to task dispensers : has_legacy_tasks and import_legacy_tasks as well as a legacy_fields attribute to workaround the fact task dispensers have no access to the descriptors to modify the files themselves. A default newly defined task dispenser will inherit these definitions but they will have no effect on the tasks. They are purposely not defined as abstract methods then.

nrybowski commented 11 months ago

Tested on 61ead04267da9c1e4c0897474d3f98c8bc73d59a with the LEPL1402.

The frontend fails while reading the course.yaml :

inginious-frontend-1      | 2023-10-11 12:15:46,690 - inginious.course_factory - INFO - Caching course LEPL1402
inginious-frontend-1      | 2023-10-11 12:15:46,777 - inginious.course.LEPL1402 - WARNING - Cannot open course
inginious-frontend-1      | Traceback (most recent call last):
inginious-frontend-1      |   File "/inginious/inginious/frontend/courses.py", line 92, in __init__
inginious-frontend-1      |     self._task_dispenser = task_dispenser_class(lambda: self._task_factory.get_all_tasks(self), self._content.get("dispenser_data", ''), database, self.get_id())
inginious-frontend-1      |   File "/inginious/inginious/frontend/task_dispensers/toc.py", line 27, in __init__
inginious-frontend-1      |     raise Exception("Invalid dispenser data structure")
inginious-frontend-1      | Exception: Invalid dispenser data structure
inginious-frontend-1      | 
inginious-frontend-1      | During handling of the above exception, another exception occurred:
inginious-frontend-1      | 
inginious-frontend-1      | Traceback (most recent call last):
inginious-frontend-1      |   File "/inginious/inginious/frontend/course_factory.py", line 126, in get_all_courses
inginious-frontend-1      |     output[courseid] = self.get_course(courseid)
inginious-frontend-1      |   File "/inginious/inginious/frontend/course_factory.py", line 52, in get_course
inginious-frontend-1      |     self._update_cache(courseid)
inginious-frontend-1      |   File "/inginious/inginious/frontend/course_factory.py", line 234, in _update_cache
inginious-frontend-1      |     Course(courseid, course_descriptor, self.get_course_fs(courseid), self._task_factory, self._plugin_manager, self._task_dispensers, self._database),
inginious-frontend-1      |   File "/inginious/inginious/frontend/courses.py", line 94, in __init__
inginious-frontend-1      |     raise Exception("Course has an invalid YAML spec: " + self.get_id())
inginious-frontend-1      | Exception: Course has an invalid YAML spec: LEPL1402

The actual raised exception is Invalid dispenser data structure.

nrybowski commented 11 months ago

Applying grouped_actions and then tasksets on top of this branch seems to solve the issue. I suppose that the tasksets patches convert old courses in a correctly formatted taskset and course.

anthonygego commented 9 months ago

Applying grouped_actions and then tasksets on top of this branch seems to solve the issue. I suppose that the tasksets patches convert old courses in a correctly formatted taskset and course.

I'll close this one and focus on #982