ttp2542 / GradingScripts

This repository contains the Python scripts used for grading in the SWEN-123 class at RIT.
4 stars 7 forks source link

Timezones #16

Open NeonixRIT opened 2 years ago

NeonixRIT commented 2 years ago

Git will handle the timezone issue automatically just by adding the '--date=local' argument to the rev-list command.

NeonixRIT commented 2 years ago

also I dont think current solution accounts for daylight savings time.

jym2584 commented 2 years ago

Hey Kamron, thanks for the heads up! The issue that I had with the timezone is with cloning the repositories since the Repository object from the pyGitHub module returns the time in GMT+0, which only clones repos that are created X (5 in this case for EST) hours past the desired pull time. If you pull at 14:00 on the same day that the repo was created, it will grab repos created past 09:00 for example.

I previously modified a conditional on the run method that accounts for the timezone issue.

I am not quite fully familiar with the rev-list parameters on git but I'll definitely be able to check this out and see what I can do! The current solution does not account for daylight savings as well, but I'll be able to take a crack at that

NeonixRIT commented 2 years ago

rev-list will get a commit hash for a commit at or before the date/time given. However, by default, if the student's time zone is -10 UTC for example, the commit hash it retrieves will be X hours ahead of the due date/time (x being the difference between the student's computer time zone and the script user's time zone), but it also automagically accounts for DST making fixing this programmatically very difficult. rev-list can handle this issue with the --date=local argument however.

In my opinion its easier to handle the repos after they're cloned, deleting ones that were created after the due date/time (will error on rollback). This method has some performance overhead though compared to the current method, but also handles time zone/DST issues.

jym2584 commented 2 years ago

Came up with a solution that still uses the datetime module, but now accounts for daylight savings.

However, by default, if the student's time zone is -10 UTC for example, the commit hash it retrieves will be X hours ahead of the due date/time (x being the difference between the student's computer time zone and the script user's time zone)

A concern that I had with timezones is primarily the cloning part unless rolling back repos also have this same issue. I don't believe this (the latter) is the case given the manual testing that I did when trying to solve the cloning issue, but it doesn't seem like it would be a difficult thing to fix given the solution that you gave. It'd be interesting if it turns out to be the case

NeonixRIT commented 1 year ago

I think I was mistaken here. Seems git accounts for time zones really well already. Your daylight savings calculation though doesnt work if they made their commit before the change to daylight savings time and grader is cloning after the change. Something unreasonable to account for. Git seems to handle this iirc.