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

nextercism: implement submit command #419

Closed
kytrinyx opened this issue Aug 4, 2017 · 12 comments
Closed

nextercism: implement submit command #419

kytrinyx opened this issue Aug 4, 2017 · 12 comments
Assignees

Comments

@kytrinyx
Copy link
Member

kytrinyx commented Aug 4, 2017

To be merged into the nextercism branch in #410

Command name: submit (alias s).
File: cmd/submit.go.

$ exercism submit
$ exercism submit path/to/directory
$ exercism submit file1 file2 [...]

Calling submit with no arguments will try to submit the current directory. If the current directory is not an identifiable exercise, we can either ask interactively what track (if multiple) and what exercise. Or we can error out and ask them to specify the path to the directory to submit.

The files are submitted as a multipart form upload as a PATCH request to the /solutions/:uuid endpoint.

The form field for the files is called files[].

--54e5745cb2d1f16f85ecbdb4115271a6b58a4ecb3a98eeb594f259bd98f6
Content-Disposition: form-data; name="files[]"; filename="/file-1.txt"
Content-Type: application/octet-stream

This is file 1.

--54e5745cb2d1f16f85ecbdb4115271a6b58a4ecb3a98eeb594f259bd98f6
Content-Disposition: form-data; name="files[]"; filename="/subdir/file-2.txt"
Content-Type: application/octet-stream

This is file 2.

--54e5745cb2d1f16f85ecbdb4115271a6b58a4ecb3a98eeb594f259bd98f6--

Given the following directory structure:

.
├── file-1.txt
└── subdir
    └── file-2.txt

The contents of the files are:

# file-1.txt
This is file 1.

# file-2.txt
This is file 2.

And the following sinatra app:

# app.rb
require 'sinatra'

post '/upload' do
  request.body.rewind
  request.body.read
end

Run the app with:

ruby app.rb

Then use this curl command to generate a multipart post:

curl -v -include --request PATCH -H "Authorization: Bearer a37dee7f-8be9-49ea-9dda-df703784607f" --form files[][email protected] --form files[]=@subdir/file-2.txt "http://localhost:4567/upload"

Note that curl strips off the subdirectory and only sends the file's basename, which is wrong.

@kytrinyx
Copy link
Member Author

kytrinyx commented Aug 4, 2017

I have an incredibly janky implementation of this locally that I will improve before letting anyone else see it.

@nikclayton
Copy link

Hi -- why was this closed?

I'd like an "exercism submit" feature that just submits the current directory without needing to specify the files, and chasing through the GitHub issues brought me to the "Nextercism" branch, and then this issue.

As far as I can tell, this feature isn't supported yet (CLI 3.0.11 at least).

Is this still on the roadmap? Is there another issue I can follow to see the progress (I wasn't able to find one)?

@NobbZ
Copy link
Member

NobbZ commented Mar 5, 2019

If at all it is a long term goal, as during a transition phase we actually had this feature and it caused a lot of bugs and superflously submitted files (eg binaries or other artifacts).

Also, this configuration needs to be available to the CLI without it needing to rely on the server to filter out the superflous files, while we do not want to have to bundle a new release of the CLI each time some naming schemes in a track change or we get a new track.

@nikclayton
Copy link

Is there another open issue for that long term goal? I couldn't find one.

A rough fix would be:

  1. Add a version number field to the metadata file. The sooner that's done the better, so that different releases of the CLI don't need to sniff the metadata fields to figure out what version it is, and can have a concrete version number to rely on instead.

  2. Update the metadata file (bump the version number) to include data about the files for each exercise that should be submitted. For example, for the JS track that's the Javascript file and the test file.

  3. Update the CLI tool, if this section is present, "exercism submit" should use it, and not allow the user to specify any files -- only the files listed should be updated (trying to specify files in this scenario should be user error, unless the user is specifying exactly the same files, in which case show the user an informational message that the file list is unnecessary).

If this section isn't present then keep the current behaviour -- require the user to specify the files to upload each time.

Possibly separate step -- if people are uploading random binaries, impose a size / type check on the server to reject them.

@kytrinyx
Copy link
Member Author

kytrinyx commented Mar 5, 2019

We have a submit command. I implemented the "submit a directory" idea, and decided I didn't like it, so I removed it. I don't have a long-term goal of supporting it.

Identifying which files are necessary to submit for any given language track or exercise is not trivial, as this is not explicitly encoded anywhere.

For example, for the JS track that's the Javascript file and the test file.

Most of the time we don't want people to submit the test file, unless they've written custom tests and want to discuss them with their mentor.

if people are uploading random binaries, impose a size / type check on the server to reject them.

Yeah, we have checks against binaries, but not against everything else.

@kytrinyx
Copy link
Member Author

kytrinyx commented Mar 5, 2019

Thinking out loud...

For most tracks, we typically only want to submit one file, and it is almost always named as either the CamelCase, snake_case, or kebab-case version of the exercise name, with the file extension tacked on.

It will sometimes be in a subdirectory, sometimes in the exercise directory itself.

What about this:

  • if it is an exercise (has the exercise metadata file in the expected location)
  • then walk the directory tree and locate all files
  • select only the files that correspond to the exercise name + a file extension
  • if that is exactly one file, submit it
  • if not, break out with a message that you need to resubmit with explicit file paths

That would cover most of the cases, and for those it doesn't cover, we could later decide whether it's worth implementing logic for them.

@nikclayton
Copy link

Is there a reason why this information can't be included in the data that the CLI downloads and saves in metadata.json, and treat that as the source of truth? I can see (in the CLI tool) where that information is fetched, but I can't see which Github repository contains the source of that data on a per-exercise basis.

The track I'm most interested in at the moment is the Javascript track, and there, most of the tests have one initial test function, and then a bunch of xtest functions that the student has to rename to test so that the test framework pays attention to them (e.g., https://github.com/exercism/javascript/blob/master/exercises/beer-song/beer-song.spec.js).

So it's important that the student remembers to upload both files.

I've been running a programming course for refugees, and when I had them using the Exercism exercises, having them remember to upload the modified test files along with the code was a source of confusion.

@NobbZ
Copy link
Member

NobbZ commented Mar 6, 2019

They do not need to upload the tests. Mentors usually know that they need to unskip the tests if they download the solution for reviewing and usually do so before running the tests, most have actually helpers that do the changes for them.

@kytrinyx
Copy link
Member Author

kytrinyx commented Mar 8, 2019

Is there a reason why this information can't be included in the data that the CLI downloads and saves in metadata.json, and treat that as the source of truth?

I'm not sure I understand what you mean by "this information".

The information about what the name of the source file is currently isn't stored anywhere at all. If we had it we could certainly add it to the metadata file. However, the only place that it exists right now is as an implicit assumption via the test suite. The file could be named anything, so there's no way to know what would have to be returned. We could make some guesses, but that's all.

@kytrinyx
Copy link
Member Author

kytrinyx commented Mar 8, 2019

They do not need to upload the tests.

Yeah, they probably should not be uploading the tests. Only in the rarest cases would that be necessary.

@kytrinyx
Copy link
Member Author

kytrinyx commented Mar 8, 2019

@nikclayton Would you mind opening an issue in http://github.com/exercism/exercism/issues about what you're trying to achieve? That way we could have a conversation about that, separate from this old (closed) issue about the original submit command.

@wilsonsilva
Copy link

Can be achieved with an alias alias exs='exercism submit ${$(basename $PWD.go)//-/_}'

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

No branches or pull requests

4 participants