submit50 v2 #15
submit50 v2 #15
Conversation
| @@ -40,7 +53,7 @@ def main(): | |||
|
|
|||
| # submit50 --checkout | |||
| elif sys.argv[1] == "--checkout": | |||
| checkout(sys.argv[1:]) | |||
| checkout(sys.argv[2:]) | |||
There was a problem hiding this comment.
Shall we add support for -c too, for consistency with -h?
| @@ -74,7 +85,7 @@ def authenticate(): | |||
| print() | |||
| break | |||
| elif ch == "\177": # DEL | |||
| if len(password) > 1: | |||
| if len(password) > 0: | |||
| _, EXCLUDE = tempfile.mkstemp() | ||
| url = "https://raw.githubusercontent.com/submit50/submit50/{}/exclude?{}".format(problem, time.time()) | ||
| url = "https://raw.githubusercontent.com/{0}/{0}/{1}/exclude".format(ORG_NAME, problem) |
There was a problem hiding this comment.
Mind Googling to see if these raw.githubusercontent.com URLs have existed for a while / are likely to? Guessing so, but would be good to know if GitHub has some fine print like "these might change at any time."
There was a problem hiding this comment.
Looks like raw.githubusercontent.com started being used in April 2014 and has been used by GitHub consistently since then. When they made the 2014 change, old links redirected to the new correct ones, so these links should likely continue to work.
| run("git config user.email {}".format(shlex.quote(email))) | ||
| run("git config user.name {}".format(shlex.quote(username))) | ||
|
|
||
| run("git symbolic-ref HEAD {}".format(shlex.quote("refs/heads/{}".format(problem)))) |
There was a problem hiding this comment.
Recall if this double formatting was necessary? I forget offhand what corner case I was worried about. Something with quotes maybe?
There was a problem hiding this comment.
Hm it doesn't seem necessary. Given that the refs/heads/ part will always be the same, I adjusted this to only require one .format().
| shlex.quote("https://{}@github.com/submit50/{}".format(username, username)), | ||
| shlex.quote(GIT_DIR) | ||
| )) | ||
| shlex.quote("https://{}@github.com/{}/{}".format(username, ORG_NAME, username)), |
There was a problem hiding this comment.
Some of these lines should be indented?
| teardown() | ||
| print(termcolor.colored("Submitted {}! See https://github.com/{}/{}/tree/{}.".format( | ||
| problem, ORG_NAME, username, problem), "green")) | ||
| print("Academic Honesty reminder: If you commit some act that is not reasonable but bring it to the attention of the course’s heads within 72 hours, the course may impose local sanctions that may include an unsatisfactory or failing grade for work submitted, but the course will not refer the matter for further disciplinary action except in cases of repeated acts.") |
There was a problem hiding this comment.
Instead of this, let's put a new prompt above the first push that asks "Keeping in mind the course's policy on academic honesty, are you sure you want to submit these files?"
| """Get one-time authentication code.""" | ||
| # send authentication request | ||
| requests.post("https://api.github.com/authorizations", auth=two_factor.auth, data={"scopes":["repo", "user"]}) |
There was a problem hiding this comment.
Will two_factor.auth definitely not be None at this point?
There was a problem hiding this comment.
Do we definitely need to provide scopes to trigger an authorization code? Is this what the github3 library does too to trigger it?
There was a problem hiding this comment.
two_factor.auth will definitely not be None. The two_factor() function is only ever called after two_factor.auth is set to (username, password).
Ah, you're right, scopes is necessary later to get the token but not strictly necessary here. Removed!
| res = requests.post("https://api.github.com/authorizations", auth=two_factor.auth, | ||
| data=data, | ||
| headers={"X-GitHub-OTP": str(code)}) | ||
| if "token" in res.json(): |
There was a problem hiding this comment.
If response is non-200, might res.json() return something other than a list?
There was a problem hiding this comment.
Definitely possible. Added a check to make sure the status code is 201 Created, per GitHub's authorizations API.
| while True: | ||
| print("Authentication Code: ", end="", flush=True) | ||
| code = input() | ||
| if code: | ||
| break | ||
| data = json.dumps({"scopes":["repo", "user"], "note":"{} {}".format(ORG_NAME, timestamp)}) |
There was a problem hiding this comment.
GitHub requires a token description (passed in as note) for a user-identifiable name for the token. Users can see those token descriptions in their settings when they log into GitHub's website.
| username = user.login | ||
| email = "{}@users.noreply.github.com".format(user.login) | ||
| two_factor.auth = (username, password) | ||
| github = github3.login(username, password, two_factor_callback=two_factor) |
|
Comments should be addressed! |
Ported
submit50tosubmit50.py. Notable differences between the two:github3.pyand more on GitHub's API directly, becausegithub3.pyrequires that every API query trigger another authentication code request (and it didn't make sense to have students input two or three different codes just to submit a problem set). Instead, this uses GitHub access tokens so that users need only enter an authentication code once.--checkoutnow supports leaving[problem]blank as well as specifying a list of usernames as command line arguments. Piping a text file with usernames one per line works too. So possible uses now are:submit50 --checkout [problem]submit50 --checkout [problem] @alice @bobcat usernames.txt | submit50 --checkout [problem]After installing the requirements in
requirements.txt, for this to work in the IDE you'll also need to execute the following to fix permissions issues: