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

Time.zone is not respected when parsing dates #1

Open
bradseefeld opened this issue Mar 8, 2012 · 4 comments
Open

Time.zone is not respected when parsing dates #1

bradseefeld opened this issue Mar 8, 2012 · 4 comments

Comments

@bradseefeld
Copy link

When I submit a query such as SELECT * WHERE created_at >= datetime 'March 08, 2012 03:00:00' I would expect the parsing of that timestamp to respect the time zone set in Time.zone.

I tried to fix this first in rgviz-rails Rgviz::Executor#column_value before realizing this was the wrong method :( The method I am looking for is Rgviz::Parser#parse_atomic_column but I am not sure how to fix it. Time.zone seems to be a rails enhancement and rgviz is supposed to work without rails. So, I think the fix should go in rgviz-rails, but I am not coming up with a good way to do it. Thoughts?

@asterite
Copy link
Owner

asterite commented Mar 8, 2012

Hmmm... it seems to be a Rails problem, I think. Take a look at this:

> rails c
Loading development environment (Rails 3.2.1)
ruby-1.9.3-p0 :001 > Time.now.zone
 => "ART" 
ruby-1.9.3-p0 :002 > Time.parse("2012-01-01").zone
 => "ART" 
ruby-1.9.3-p0 :003 > Time.zone
 => (GMT+00:00) UTC 
ruby-1.9.3-p0 :002 > Time.zone.parse("2012-01-01").zone
 => "UTC" 

I see this in config/application.rb:

# Set Time.zone default to the specified zone and make Active Record auto-convert to this zone.
# Run "rake -D time" for a list of tasks for finding time zone names. Default is UTC.
# config.time_zone = 'Central Time (US & Canada)'

Shouldn't the default be what Time.now.zone returns instead of UTC?

What you can do, for your specific app, is to set the TZ environment variable:

> TZ=UTC rails c
Loading development environment (Rails 3.2.1)
ruby-1.9.3-p0 :001 > Time.now.zone
 => "UTC" 
ruby-1.9.3-p0 :002 > Time.parse("2012-01-01")
 => 2012-01-01 00:00:00 +0000 

It's bad that you have to specify it in the environment variable and in the Rails configuration, though.

Do you think setting the env var is enough, or we think of something else? Maybe put the date/time parsing logic in public, overridable methods in Rgviz::Parser, and let rgviz-rails override them. I think defining a configuration for this might be overkill. :-P

What do you think?

Also, can I put your name, Brad Seefeld, in the list of contributors? :-)

@bradseefeld
Copy link
Author

Yes, you may put my name in the contributors.

I am not so sure its a Rails problem. I am still getting accustomed to how they manage time zones, but I think its the expected behavior. How I understand it:

Time.now # Returns time in _system_ time zone
Time.zone.now # Returns time in _application_ time zone
Time.parse # Parses time assuming given string is in _system_ time zone
Time.zone.parse # Parses time assuming given string in _application_ time zone

So what I would want to do in Rgviz::Parser is change Time.parse to Time.zone.parse. The problem is, Time.zone seems to be a Rails enhancement. Perhaps the simplest approach is to use Time.zone.parse if Time.respond_to?(:zone) I didnt think of that before, but it seems like an acceptable approach.

@asterite
Copy link
Owner

asterite commented Mar 8, 2012

Checking on Time.respond_to?(:zone) could work, I guess. My fear is that if there's another framework out there has another way of configuring and parsing time zones, we'll again need to change the code. And what if you are not using Rails and somebody declares Time.zone to something else, that doesn't have the parse method?

I think the safest (though maybe not cleanest) solution is to define the parsing as methods which can be overriten, and override them in rgviz-rails. If another framework does it in another way, it just needs to override that method, no need to change the code again.

@bradseefeld
Copy link
Author

OK, agreed. I am working on it now.

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