search

fraschetti / Octoslack

OctoPrint plugin for Slack, Mattermost, Pushbullet, Pushover, Rocket.Chat, Discord, Riot/Matrix, & Microsoft Teams
MIT License
74 stars 34 forks source link

fix date math with a local TZ #46

Closed tedder closed 5 years ago

tedder commented 5 years ago

Having a local TZ gives the right ETA, but the day it would finish was wrong- so often it would say "finishes tomorrow 7pm" rather than "finishes 7pm today", basically. This fixes that.

tedder commented 5 years ago

Um, something is wrong here, because I just saw an update that will finish "yesterday". I'm gonna work on this.

fraschetti commented 5 years ago

Hi @tedder The root cause here is the humanize library - it always compares the date we pass in to date.today() and that won't take into account the eta time zone.

It doesn't look like humanize is going to work here. Luckily that concept is extremely simple and won't be hard to rewrite. I should be able to knock this out soon.

tedder commented 5 years ago

Thanks. If you want I'll deal with it, depends on your motivation. I still think there was some "double conversion" happening, but even my fixes are wrong, so I'm glad you looked at humanize!

fraschetti commented 5 years ago

Hi @tedder

I submitted a fix that seems to work for all my tests. Feel free to give it a try and let me know how it goes.

https://github.com/fraschetti/Octoslack/commit/6952627bb0522e456c36008d65a17008432f7197

tedder commented 5 years ago

I am away from my printers a couple of days, I'll test it out then. Going to reopen this so I don't lose it.

tedder commented 5 years ago

Did "normal" testing and a "torture test" that is far in the future. Seems fine.

screenshot from 2019-02-20 15-04-32 screenshot from 2019-02-20 15-03-51

The following screenshots show a fairly normal job; note that 1600 is midnight UTC, which is when I saw all the crazy date math problems.

screenshot from 2019-02-20 15-25-48

(I know this pull isn't "correct", just communicating it here because that's where the discussion is)

fraschetti commented 5 years ago

Thanks @tedder - I appreciate you taking the time to validate the fix.