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 official+unofficial submit #190

Merged
merged 9 commits into from
Jan 27, 2015
Merged

Conversation

icanb
Copy link
Member

@icanb icanb commented Jan 26, 2015

No description provided.

@icanb icanb added this to the S15 Semester milestone Jan 26, 2015
@icanb
Copy link
Member Author

icanb commented Jan 26, 2015

Fixes issue #126

@@ -91,6 +91,8 @@ def saveFile(upload)
# Sanity!
upload['file'].rewind
File.open(path,"wb") { |f| f.write(upload['file'].read)}
elsif upload['local_submit_file']
File.open(path,"wb") { |f| f.write(IO.read(upload['local_submit_file']))}
Copy link
Member

Choose a reason for hiding this comment

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

Why are there two different methods for reading the file here?

Copy link
Member Author

Choose a reason for hiding this comment

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

upload['file'] is a class that we're not using in the case of local submit. so there is not way to do .read

Copy link
Member

Choose a reason for hiding this comment

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

So upload['file'] is an actually file-like object, but upload['local_submit_file'] is a string? Could we get a comment for that.

@dlbucci
Copy link
Member

dlbucci commented Jan 26, 2015

If we're gonna say this fixes #126, this should fix the scripts/autolabSubmit.sh too.

@icanb
Copy link
Member Author

icanb commented Jan 26, 2015

Is 213 using that script? Otherwise, can we just get rid of it?

@dlbucci
Copy link
Member

dlbucci commented Jan 26, 2015

At least 122 and 210 are using that script for command line submission. We should have an official local submit script.

@metalogical
Copy link

We (210) actually have our own script that follows the protocol, so as long as the protocol is documented, getting rid of the official script is perfectly fine. I believe that 122 does a similar thing (@robsimmons).

ymzong pushed a commit that referenced this pull request Jan 27, 2015
@ymzong ymzong merged commit 4d7340e into master Jan 27, 2015
@ymzong ymzong deleted the fix_official+unofficial_submit branch January 27, 2015 15:56
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.

4 participants