search

bitwalker / timex_ecto

An adapter for using Timex DateTimes with Ecto
MIT License
162 stars 68 forks source link

RFC: Force conversion to/from a specific timezone #47

Open phillipp opened 8 years ago

phillipp commented 8 years ago

Hi, I'm maintaining a legacy app where the time zones are stored in the local time zone instead of UTC. I want to access the database via ecto. The problem is that it messes up the time by converting it to/from UTC and so everything breaks (there are literally hundreds of other clients that use the same database).

What is the best way to turn off the time zone conversion?

phillipp commented 8 years ago

I have implemented such behaviour in my fork (hard-coded for the time being: https://github.com/phillipp/timex_ecto/commit/475d0be10bab2882e404e7fa6241387bc000c3bf). Would you like to integrate that? In that case, I'm not so sure what the best implementation would be. I thought about an extra type (like Timex.Ecto.LocalDateTime or sth.) but 99% of the module would be exactly the same code than the DateTime type. Can you think of a better way? I'm new to elixir and haven't fully grasped all the patterns that are possible and when to use what.

bitwalker commented 8 years ago

Probably the best way is to implement a Timex.Ecto.NaiveDateTime type which performs no conversions. Inputs will be converted to NaiveDateTime, and outputs will always be NaiveDateTime.

bitwalker commented 8 years ago

It's implementation can be very similar to Timex.Ecto.DateTime, just without the conversions to/from UTC.

bitwalker commented 8 years ago

I will put this on my TODO list, but it may be a week or two before I can get to it, if you'd like to take a stab at it and open a PR to collaborate on, I'd be glad to help with any questions.

phillipp commented 8 years ago

I have now tested two implementations:

a) one that converts the time from/to the local timezone, and that worked as I expected b) one "naive" that does not convert before/after storing. This works for my own timestamps from the application that I converted explicitly to the local timezone before storing but not for the inserted_at and updated_at timestamps as they seem to be always generated in the UTC time zone.

At least for my use case I would need the first implementation. I'm not sure if that is a extremely specific case, but coming from Rails I wanted to match this behaviour:

    config.time_zone = 'Berlin'

    config.active_record.default_timezone = :local
    config.active_record.time_zone_aware_attributes = false

What do you think, is that too application-specific and should be part of my app or might there be a use case that should be supported by the lib? I think both might be possible...

bitwalker commented 7 years ago

Sorry it took me so long to get back to this - I am open to support such configuration, but I would suggest proposing it to the Ecto team first - it would be far better if it was an upstream config option, but if it's something they are not prepared to do in Ecto, I would be open to a PR for this project :)