fix issue #1732

[jlohse-mpi-sws]

No description provided.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.83%. Comparing base (3277ca5) to head (75f190e).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
cms/grading/tasktypes/interactive_keeper.py	0.00%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1733      +/-   ##
==========================================
+ Coverage   56.72%   56.83%   +0.11%     
==========================================
  Files         344      344              
  Lines       28314    28363      +49     
==========================================
+ Hits        16060    16121      +61     
+ Misses      12254    12242      -12     

Flag	Coverage Δ
functionaltests	0.00% <0.00%> (ø)
unittests	56.83% <0.00%> (+0.11%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[veluca93]

Looks like some test was relying on the previous behaviour, could you check?

[jlohse-mpi-sws]

Yes sorry. The issue was that my change prioritized controller failures over contestant failures, but it has to be the other way around in case the controller fails because of waiting for the contestant.

[fagu]

If the controller fails because of waiting for the contestant, doesn't that indicate a bug in the controller or a too-low controller wall time limit? So I would expect "controller failed" in this case.
Also, it seems reasonable to me that the submission is allowed to crash if the controller failed. (Or are we expecting the submission stubs to detect incomplete inputs in all cases?)

[fagu]

Somewhat off-topic: Are the sandboxes intentionally not cleaned up in the "invalid command from controller" branch?
https://github.com/jlohse-mpi-sws/cms/blob/75f190ed042b1b5c92a13cab6918791c193d2ee0/cms/grading/tasktypes/interactive_keeper.py#L183-L191

[jlohse-mpi-sws]

If the controller fails because of waiting for the contestant, doesn't that indicate a bug in the controller or a too-low controller wall time limit? So I would expect "controller failed" in this case. Also, it seems reasonable to me that the submission is allowed to crash if the controller failed. (Or are we expecting the submission stubs to detect incomplete inputs in all cases?)

In the current implementation there is only one wall time limit (controller_wall_limit) and the contestant gets the same wall limit. Therefore, if the contestant sleeps, the controller (who is started first) will likely time out first.

People have been saying it is too hard to reliably find out in general whether the controller died because the contestant took too long, it guess this is why it is implemented like this @veluca93 ?

[fagu]

Thanks for your explanation. I hadn't realized that the solution and the controller have the same wall time limit.

So with the current implementation, I suppose a solution can force (success=False,"Controller Failed") by sleeping for a little less than the wall time limit and then quitting. (Not sure how problematic this is if done intentionally, but couldn't something to this effect also happen accidentally and then the participant would never learn the result of their submission?)

This implementation also means that the stub has to check every input from the controller for completeness before handing it to the contestant's code as the controller could have been killed while writing, right?