search

hammerlab / cycledash

Variant Caller Analysis Dashboard and Data Management System
Other
36 stars 2 forks source link

Suggest using UTC timezone for better compatibility with timestamps #819

Closed armish closed 9 years ago

armish commented 9 years ago

I recently wiped my develop environment on Mac OS X 10.10 and with that came a new PostgreSQL installation. Everything was working fine but the seltests were failing due to different timestamps on the examine pages for various test runs.

I was able to resolve this issue by switching my default timezone within postgresql.conf and thought that this was something specific to my setup; but, looks like issue #727 is also related to this and mentioning this within our DEVELOP.md helps clarifying this issue.

Review on Reviewable

ihodes commented 9 years ago

Hmm seltest tests should have all timestamps blanked out; which pages were timestamps showing up on?

ihodes commented 9 years ago

Ah I see, this is from the Examine page timestamps?

armish commented 9 years ago

yeah, the seltests for the Examine page were mysteriously failing me although everything looked the same... well, except the timestamps ;) This fixed the problem for me, not sure if there is a better way to fix this once and for all.

ihodes commented 9 years ago

I just checked my database and my timezone is US/Eastern.... there must be something else doing on here?

armish commented 9 years ago

hmm, that's interesting. Then setting the timezone is just a hack and it is possible that the problem I had was different than #727.

Do you think this new timezone=True has something to do with the issue I was having?

ihodes commented 9 years ago

Yeah I'm really not sure what's going on here, @tavinathanson has a fair amount of experience with this, may be worth chatting. What is timezone=True?

tavinathanson commented 9 years ago

Sorry for not chiming in earlier; didn't see this PR.

@ihodes, I'm confused about how your DB is Eastern and still working correctly! My local, and dev1, are both UTC (just double checked).

I'm going to set up CycleDash right now and see what happens when I mess with UTC vs. Eastern in the conf.

tavinathanson commented 9 years ago

Hmm. I'm currently making comments (locally), switching the Postgres setting between UTC and US/Eastern, making more comments, repeat. And all the comments are showing up correctly ("a few seconds ago", "2 minutes ago", etc.). So that doesn't seem to be the issue (or even necessary?).

Speaking of whether or not the conf change is necessary, I appear to have said in #576: " (Though, that UTC setting in the conf file doesn't appear to be necessary, per: "it is assumed to be in the time zone indicated by the system's timezone parameter, and is converted to UTC using the offset for the timezone zone".)" (The inner quote is from http://www.postgresql.org/docs/9.1/static/datatype-datetime.html)

@armish, could your OS X timezone have gotten messed with?

armish commented 9 years ago

thanks for checking this @tavinathanson!

This is what I have for my timezone settings: screen shot 2015-09-01 at 3 18 36 pm

Maybe this is yet another issue with my local machine and not a broad one (I seem to have many similar issues for some reason). Let's dismiss this PR until somebody else can replicate this problem.

With that, is it now safe to close #727 as well?

ihodes commented 9 years ago

Re #727, unless we've fixed it, let's keep it open.

tavinathanson commented 9 years ago

Re-opening because changing to UTC fixes the issue for both @jaclynperrone and @armish. @ihodes, what's different about your setup?!

ihodes commented 9 years ago

Crazy! Maybe I do have the issue and have never noticed it? Either way, thanks for tracking it down.

tavinathanson commented 9 years ago

Can you try creating a new comment to check?

ihodes commented 9 years ago

It all appears to work for me! Maybe my postgres isn't using that config? http://link.isaachodes.io/image/301u080H0z1g

jaclynperrone commented 9 years ago

@ihodes Is the timestamp also correct on the Runs page? It was wrong for me in the "Recent Comments" section.

tavinathanson commented 9 years ago

So the two remaining mysteries are why non-UTC works for @ihodes and why it also works for me when I switch away from UTC. I guess that's not quite resolved.

ihodes commented 9 years ago

(Off-thread we discovered that if I refreshed the page, the timestamp is out of date).

Hmm what do you mean "works"? It seems to not work given the refreshing showing an incorrect date?

tavinathanson commented 9 years ago

Aha! So @ihodes's doesn't work after a refresh, when it actually pulls from the DB. Given that, I think this change should go in. Any objections?

Edit: beaten to the punch.

Edit again: I meant "works to produce the error." Very unclear. My bad.

ihodes commented 9 years ago

Ahh got it :) Excellent. Thank for tracking this down!

I'm still puzzled at how seltests were failing because of this (I haven't run them very recently, but I haven't had a timestamp issue with them in the recent past).

armish commented 9 years ago

@ihodes: this is still a wild guess, but I think our schema change made those tests fail for me on examine pages. I didn't have this issue when were simply doing < cycledash.sql.

ihodes commented 9 years ago

Weird—I will try running the seltest tests tomorrow to see! In the meantime though, please merge on green, and sorry for the trouble!

armish commented 9 years ago

no worries -- I have been having weird issues with my local setup lately, so I wasn't really sure whether this was because of my setup or not.

Merging this one in.