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

Timezones #16

Open
NeonixRIT opened this issue Mar 23, 2022 · 5 comments
Open

Timezones #16

NeonixRIT opened this issue Mar 23, 2022 · 5 comments

Comments

@NeonixRIT
Copy link
Contributor

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

@NeonixRIT
Copy link
Contributor Author

NeonixRIT commented Mar 23, 2022

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

@NeonixRIT NeonixRIT reopened this Mar 23, 2022
@jym2584
Copy link
Collaborator

jym2584 commented Mar 23, 2022

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
Copy link
Contributor Author

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 added a commit that referenced this issue Mar 24, 2022
Addresses #16 for daylight saving issue
@jym2584
Copy link
Collaborator

jym2584 commented Mar 25, 2022

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
Copy link
Contributor Author

NeonixRIT commented Sep 12, 2022

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.

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

2 participants