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

Apply localTimeZone offset when reading DateAndTime instances #10

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

rko281
Copy link
Contributor

@rko281 rko281 commented Apr 8, 2020

Also fix associated unit test to avoid permanently changing the image's time zone. Closes #9.

…ix associated unit test to avoid permanently changing the image's time zone. Closes #9.
@tblanchard
Copy link
Contributor

I think this bakes in the assumption that the database will be running with a GMT offset and that the client has a sensible localtime setup. This is not always the case. DateAndTime should be in server time regardless. Converting to a localtime is a UI issue.

I'm leaving it open because I'm willing to be convinced but I have one app for a local tourist attraction and db and web app are both running in the tz of the attraction and not GMT.

@daniels220
Copy link

Agreed, baking in the assumption that the client has a sensible localtime is a problem, because there is no such thing as a sensible localtime when handling values far from now. It's the same problem that Windows has with file mod dates—a time from when DST was in effect will be off by an hour when read with DST not in effect and vice-versa. For applications that require correctness here, the approach I'm aware of is to treat the value in the DB as either UTC or local-time-as-displayed (up to the application developer), then compute the correct offset using a more advanced timezone library on retrieval. In either case, the arguably correct behavior would be to return a DateAndTime with an offset of nil, not-yet-known, but this is not supported by the DateAndTime class and would cause errors.

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

Successfully merging this pull request may close these issues.

DateAndTime to/from DATETIME is inconsistent
3 participants