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

Set application timezone for zoneless datestamps #95

Closed
wants to merge 1 commit into from

Conversation

untergeek
Copy link
Contributor

Fortunately, the Sequel gem already had the necessary functionality.

@untergeek untergeek force-pushed the fix/89 branch 3 times, most recently from e1807c0 to 8e9ce6d Compare December 8, 2015 00:35
@@ -65,6 +67,14 @@ def setup_jdbc_config
# The amount of seconds to wait to acquire a connection before raising a PoolTimeoutError (default 5)
config :jdbc_pool_timeout, :validate => :number, :default => 5

# Timezone conversion.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these docs do a great job describing the behaviors around timezones, but it would be great to be clearer around what setting this parameter will achieve, instead of just the differing behaviors of timezone management by the plugin

Fortunately, the Sequel gem already had the necessary functionality.

However, it turns out that Sequel _can_ do named timezones, but when you do
the date comes back as a DateTime object instead.  As a result,
this should only use named_timezone support if defined, which should
save cycles for those who already have their date fields in UTC, rather
than punishing every user.

Use string date to test timezone conversion to guarantee similarity to the
issue case
@talevy
Copy link
Contributor

talevy commented Dec 8, 2015

LGTM

@elasticsearch-bot
Copy link

Aaron Mildenstein merged this into the following branches!

Branch Commits
master 2b439cb

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.

3 participants