search

fedora-infra / fas

Fedora Account System
https://admin.fedoraproject.org/accounts
GNU General Public License v2.0
40 stars 50 forks source link

Fix login #196

Closed skrzepto closed 8 years ago

skrzepto commented 8 years ago

Modifying sqlalchmey naive datetime to utcdatetime so that login will work

Issue #195

skrzepto commented 8 years ago

This will also allow my testsuite to pass

laxathom commented 8 years ago

Hey, well you don't need actually to abstract the datetime to handle utc there. DateTime handles it already with param timezone :wink: That's the fix I have locally on @pypingou 's branch. I think we can close that PR

skrzepto commented 8 years ago

@laxathom I tried DateTime(timezone=True) and it didn't work for me :/ maybe I forgot to fix the admin script.

Could we get that fix into @pypingou branch before we merge/close the pr or could you open a new pr with that fix so that working code is always getting pushed into the repo :)

laxathom commented 8 years ago

That's b/c you have a previous stored naive-offset datetime. Did you rebuild the db from scratch? Also, I did remove the non-sense default value on login_timestamp as when the user get created, it actually never logged in yet.

skrzepto commented 8 years ago

@laxathom updated this PR to reflect your comments. It works on my machine

edit

actually after logging in again this won't work. I'm wondering if my initial approach was valid.

skrzepto commented 8 years ago

Okay there are two methods I got the original implementation from

http://stackoverflow.com/questions/2528189/can-sqlalchemy-datetime-objects-only-be-naive/2528453#2528453

But there seems to be a lib for this as well

https://pypi.python.org/pypi/SQLAlchemy-UTCDateTime/

which way should we go?