Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix viewing testrun results (issue #123) #124

Merged
merged 3 commits into from
Mar 6, 2023

Conversation

A-dead-pixel
Copy link
Contributor

@A-dead-pixel A-dead-pixel commented Dec 20, 2022

Fixes #123.

Make the testrun controller check whether the input file is a zip by extension only and save that information in the model.
As submitted input zips are correctly validated for containing exactly one file in sioworkers anyway in sio.executors.common._run, the testrun module doesn't have to do that thoroughly.

Without this PR, the file was checked every time it's submission was viewed and the lack of a seek method in filetracker seems to have been causing errors.
A detailed traceback can be viewed in the issue.

Additionally, now when a zipped input file is being downloaded, it will have a .in.zip extension, as opposed to just .in.

Make the testrun controller check whether
the input file is a zip by extension only
and saves that information in the model.

Change a downloaded zipped input file's name,
so it has a .zip extension.
@A-dead-pixel A-dead-pixel changed the title Fix for issue #123 Fix viewing testrun results (issue #123) Dec 28, 2022
twalen
twalen previously approved these changes Jan 9, 2023
return stream_file(submission.input_file, name='input.in')
name='input.in'
if submission.input_is_zip:
name+='.zip'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would return "input.in.zip", what about using "input.in" for regular inputs and "input.zip" for zip files?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My rationale was that that after unzipping the user should get the same filename as the one for uncompressed input, but now I see that the zipfile contains the input in a user-named file, so it's irrelevant.

While we're at it, would you agree that the filename should contain some additional information like it does for normal ins and outs (problems_1_rok1a.in/.out)?
I'd go with something like testruns_{submission_id}_{short_problem_name}.in/.zip/.out.

from django.db import migrations, models


class Migration(migrations.Migration):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is data migration needed? if not I suggest to add some comment in the code why it can be skipped.

@A-dead-pixel A-dead-pixel marked this pull request as draft February 25, 2023 19:56
@A-dead-pixel
Copy link
Contributor Author

A-dead-pixel commented Mar 5, 2023

I reverted the previous commit, since by utilizing some changes from #151 (thanks @top-sekret!), we can perform full-fledged zipfile checks without messy code. Additionally, now users can't bypass the 50KiB zipped input limit by removing the .zip extension.
Regarding the input filenames, for now I'm leaving them at input.zip and input.in.

@A-dead-pixel A-dead-pixel marked this pull request as ready for review March 5, 2023 13:10
@twalen twalen merged commit 13104de into sio2project:master Mar 6, 2023
@A-dead-pixel A-dead-pixel deleted the Testrun-PR branch March 6, 2023 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't view testrun result
2 participants