INGInious icon indicating copy to clipboard operation
INGInious copied to clipboard

YAML order is not preserved

Open mlhetland opened this issue 5 years ago • 6 comments

From what I can tell, the intended behavior of INGInious is that the order of YAML fields should be preserved between reading and writing? At least, it seems that that's what the custom_yaml module is trying to achieve. This is also something I'd highly appreciate, as it would make it much easier to merge git branches where the YAML files have been modified, for example.

However, from what I can tell, it's not working. We're currently using the v0.5 branch, and when the YAML entries shift arond a bit arbitrarily, just like you'd expect if a dict were used (as in the original PyYAML), rather than an OrderedDict. It seems to me, then, that this is a bug? Or?

mlhetland avatar Oct 05 '19 09:10 mlhetland

There may actually be some code somewhere that unintentionally cast the OrderedDict to a simple dict (typically a comprehension). When do you observe this exactly ?

anthonygego avatar Oct 07 '19 06:10 anthonygego

It's quite pervasive, whenever a task.yaml is edited via the front-end, which is why we initially assumed there was no order preservation at all. However, looking through our commit history, it seems to me that it does not affect top-level maps, but all nested ones. If that is the case (not 100% yet), maybe it's an issue of the custom loading or serialization not being passed along beyond the top level?

If you don't see the behavior at your end, I could try to set up a minimal example, but as I said, whenever we edit something, the YAML files are shuffled around, so unless it's only on our end, you should be able to observe it easily enough.

mlhetland avatar Oct 07 '19 07:10 mlhetland

OK, here's a simpe reproduction procedure, which indicates that it actually happens even at the top level:

  1. Create new task demo with default settings.
  2. Add code subproblem demo with default settings.
  3. Save changes.

This now includes the following:

input_random: '0'
limits:
    memory: '100'
    time: '30'
    output: '2'
  1. Swap the lines with input_random and limits manually, with a text editor.
  2. Reload web front-end and save changes.

At least in my current session, this consistently rewrites the YAML file to the order it had.

Of course, the actual order might vary arbitrarily (given the behavior of dicts), but I guess the behavior might be the same.

Now, normally the issue comes from editing the files solely via the web-frontend (e.g., in different sessions/on different machines), not in a text editor like this.

I suppose one reason one might not notice it that much is if one runs a single server, and only edit with that, in a long-running session, as the dicts would have the same (arbitrary) order for the same keys, perhaps?

mlhetland avatar Oct 07 '19 07:10 mlhetland

I’d be happy to help (and have others to assist), if you have suggestions for pinpointing/debugging. An option is, of course, to simply pore over the code for a few hours… :-}

mlhetland avatar Oct 10 '19 15:10 mlhetland

I have been looking into this and it seems like the problem lies in the task_edit.py file. Here, in the POST_AUTH function, data is inserted into the dictionary (or storage object) inside the web.py library call, which seems to not consider the original order from the task file. Further, from what I can deduce, it seems like the code just removes the original file and write the new file from this new dictionary (storage object).

So, except for the order of the exercises and the order of the answers, it seems like the edit page does not consider the original order in the task file. This probably means that to mitigate this problem, one would have to load the original data first, the update the original dictionary with the new changes and the save this dictionary.

I can also mention that the order of the output seems to stay the same within one running session of the webserver. However, each time the webserver is restarted, the order seems to change.

zootos avatar Oct 25 '19 14:10 zootos

Linked to #393

anthonygego avatar Sep 28 '21 12:09 anthonygego