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

Send Tango unambiguous filenames for user handins #1947

Merged
merged 4 commits into from
Jul 21, 2023

Conversation

cg2v
Copy link
Member

@cg2v cg2v commented Jul 18, 2023

Summary

Summary generated by Reviewpad on 18 Jul 23 17:15 UTC

This pull request includes three patches:

  1. The first patch exports a new method handin_file_long_filename in the Submission model, which generates a long filename for the handin file by including the student's email address.

  2. The second patch allows the autogradeInputFiles method in the assessment_autograde_core helper to provide a remoteFile attribute for autograding file uploads to the Tango server. If remoteFile is not provided, the value of localFile will be used instead.

  3. The third patch updates the autogradeInputFiles method in the assessment_autograde_core helper to use the full filename (including the student's email address) for handin file uploads to the Tango server.

Please review these patches for implementation correctness and possible improvements.

Description

Tango handin filenames need to be unique per user/version (and also course/assessment) so that grading always applies to the student's work and not someone elses. This patch restores the upload filename to <email>_<version>_<handin>.<ext> from <version>_<handin>.<ext>

Motivation and Context

Since #1831 , autolab has been sending tango filenames of the form <version>_<handin>.<ext>. This means that if multiple students submit near-simultaneously, and they are at the the same version number, it is likely that the grade both receive will be based on only one of their handins.

How Has This Been Tested?

Deployed to autolab-dev.andrew.cmu.edu
Submitted new handin for https://autolab-dev.andrew.cmu.edu/courses/TestCourse-s23/assessments/hello
Verified that the files uploaded to tango had the email address in them for both new handins and regrades.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have run rubocop for style check. If you haven't, run overcommit --install && overcommit --sign to use pre-commit hook for linting
  • My change requires a change to the documentation, which is located at Autolab Docs
  • I have updated the documentation accordingly, included in this PR

Other issues / help required

If someone is working on an alternative approach, feel free to reject this PR

cg2v added 2 commits July 18, 2023 13:03
Allow autogradeInputFiles to provide a remoteFile attribute that will
be used for the filename on the tango server. If remoteFile is not
provided, the value of localFile will be used.
@reviewpad reviewpad bot requested a review from najclark July 18, 2023 17:15
@reviewpad reviewpad bot added small Pull request is small waiting-for-review labels Jul 18, 2023
cg2v added 2 commits July 18, 2023 14:14
When uploading handins to tango, send the long filename (with the
student's email address) as the filename to use on the tango server
Unlike localFile, remoteFile can always be a basename, so remove
all the excess calls to File.basename
@cg2v cg2v force-pushed the tango_unambiguous_filenames branch from 7c151fc to 553d770 Compare July 18, 2023 18:16
@damianhxy
Copy link
Member

Thanks for the PR! (I presume you meant to reference #1831 instead)

@cg2v
Copy link
Member Author

cg2v commented Jul 18, 2023

Now tested on autolab-dev. Seems to work

root@autolab-tango-d01 /]# ls /home/autolab/courselabs/KEY-TestCourse-s23-hello/
1_hello.c  4_hello.c  autograde-Makefile             [email protected]_2_hello.c
2_hello.c  5_hello.c  autograde.tar                  [email protected]_3_hello.c
3_hello.c  6_hello.c  [email protected]_1_hello.c  output

@cg2v
Copy link
Member Author

cg2v commented Jul 18, 2023

Reference and testing status updated in summary

Copy link
Contributor

@20wildmanj 20wildmanj left a comment

Choose a reason for hiding this comment

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

Reproduced submission error locally through autolab, and verified that fix works correctly:

Reproduction Strategy (on master):

  • Use this autograded assessment (takes in a python file, returns score based on number function you define spits out):
    tangotestlab.zip, handin:
    returnNum.py.txt

  • Submit as different users different values, get a list of submissions like so:
    Screenshot 2023-07-19 at 9 34 46 PM

  • Go to "Manage Submissions" and click "Regrade All"

  • See that when regrade finishes, all students have same score (seems to always be the last user's submission)
    Screenshot 2023-07-19 at 9 35 42 PM

Checked out branch, restarted autolab, and regraded, saw that grades matched expected values for students:

Screenshot 2023-07-19 at 9 39 15 PM

Fix LGTM, thanks for the work you did to solve our issue!

@20wildmanj 20wildmanj added this pull request to the merge queue Jul 21, 2023
Merged via the queue into autolab:master with commit 557746a Jul 21, 2023
6 checks passed
NicholasMy pushed a commit to UB-CSE-IT/Autolab that referenced this pull request Sep 13, 2023
* Export long handin filename

* Allow autogradeInputFiles to provide remote name

Allow autogradeInputFiles to provide a remoteFile attribute that will
be used for the filename on the tango server. If remoteFile is not
provided, the value of localFile will be used.

* Use full filename for tango handin files.

When uploading handins to tango, send the long filename (with the
student's email address) as the filename to use on the tango server

* sendJob remoteFile is always a basename

Unlike localFile, remoteFile can always be a basename, so remove
all the excess calls to File.basename

(cherry picked from commit 557746a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
small Pull request is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants