Time.zone is not respected when parsing dates #1

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

Comments

Projects
None yet
2 participants
@bradseefeld

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

This comment has been minimized.

Show comment Hide comment
@asterite

asterite Mar 8, 2012

Owner

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? :-)

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

This comment has been minimized.

Show comment Hide comment
@bradseefeld

bradseefeld Mar 8, 2012

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.

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

This comment has been minimized.

Show comment Hide comment
@asterite

asterite Mar 8, 2012

Owner

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.

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

This comment has been minimized.

Show comment Hide comment
@bradseefeld

bradseefeld Mar 8, 2012

OK, agreed. I am working on it now.

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