search

kfl / staffeli_nt

Staffeli NT Technology
MIT License
8 stars 7 forks source link

Fix for handins with multiple zip files #44

Closed Cavtheman closed 2 years ago

Cavtheman commented 2 years ago

Currently the download.py script crashes if students hand in more than one zip file as it attempts to unpack both. I only have time to add a small workaround in the form of a --no-unzip flag but think it would be a good idea to handle it in a better way.

kfl commented 2 years ago

What do you mean by crashes? The code was extended to unzip all files at one point, because a TA convinced me that was the right behaviour.

Crashes are of cause not acceptable, and should be fixed. However rather than adding a command-line option as a patch I'd have an analysis of what the right/a good behaviour is.

Cavtheman commented 2 years ago

When a zip file is unzipped, the script creates a folder named "unpacked". When multiple zip files are handed in, this is done for every one of them, causing the program to crash with this error: FileExistsError: [Errno 17] File exists: 'Assignment/kuid/unpacked'

kfl commented 2 years ago

Ah, now I remember. It used to be the case that staffeli would only unzip files called handin.zip and code.zip (and it would be sad if a student handed in both a handin.zip and a code.zip).

But it seems that @Spatenheinz removed that check when he implemented the OnlineTA functionality.

So I'm not sure what's the right behaviour, short term. I don't like adding a --no-unzip flag, because it will break the onlineTA functionality. At the very least the the program should complain if you are trying both the --no-unzip functionality together with having onlineTA in your grade.yml file. I think the right fix is either to catch the FileExistsError from os.mkdir and unzip everything into the unpacked dir, or to never unpack zips if there are multiple zip attachments.

Long term, this just emphasise why it's important to find a good solution for issue #4.

kfl commented 2 years ago

Superseded by PR #56