jfinkels / flask-restless

NO LONGER MAINTAINED - A Flask extension for creating simple ReSTful JSON APIs from SQLAlchemy models.
https://flask-restless.readthedocs.io
GNU Affero General Public License v3.0
1.02k stars 301 forks source link

How can I fix TypeError: can't compare offset-naive and offset-aware datetimes #630

Open green3g opened 7 years ago

green3g commented 7 years ago

Running into this issue if I have a model like this:

class Plan(db.Model):
    id = db.Column(db.Integer, primary_key=True)
    plan_date = db.Column(db.DateTime)

And I send a request to flask-restless like this:

PATCH /api/plan_index/3936

{"data":{"attributes":{"plan_date":"2017-01-24T06:00:00.000Z"},"type":"plan_index","id":"3936"}}

It only happens once there is a value set on that column. If the value is null, and I set a new datetime, the PATCH is successful.

Also, posting new features with values is successful. It is only updating existing datetime values.

I'm not sure if this is a problem with flask-restless or not, but it seems that flask-restless is the first logical step to solving it.

  File "c:\\www\\flask_app\\virtualenv\\src\\flask-restless\flask_restless\views\base.py", line 444, in wrapped
    return func(*args, **kw)
  File "c:\\www\\flask_app\\virtualenv\\src\\flask-restless\flask_restless\views\resources.py", line 707, in patch
    result = self._update_instance(instance, data, resource_id)
  File "c:\\www\\flask_app\\virtualenv\\src\\flask-restless\flask_restless\views\resources.py", line 640, in _update_instance
    self.session.flush()
  File "c:\python27\lib\site-packages\sqlalchemy\orm\scoping.py", line 157, in do
    return getattr(self.registry(), name)(*args, **kwargs)
  File "c:\python27\lib\site-packages\sqlalchemy\orm\session.py", line 2137, in flush
    self._flush(objects)
  File "c:\python27\lib\site-packages\sqlalchemy\orm\session.py", line 2257, in _flush
    transaction.rollback(_capture_exception=True)
  File "c:\python27\lib\site-packages\sqlalchemy\util\langhelpers.py", line 60, in __exit__
    compat.reraise(exc_type, exc_value, exc_tb)
  File "c:\python27\lib\site-packages\sqlalchemy\orm\session.py", line 2221, in _flush
    flush_context.execute()
  File "c:\python27\lib\site-packages\sqlalchemy\orm\unitofwork.py", line 389, in execute
    rec.execute(self)
  File "c:\python27\lib\site-packages\sqlalchemy\orm\unitofwork.py", line 548, in execute
    uow
  File "c:\python27\lib\site-packages\sqlalchemy\orm\persistence.py", line 177, in save_obj
    mapper, table, update)
  File "c:\python27\lib\site-packages\sqlalchemy\orm\persistence.py", line 659, in _emit_update_statements
    lambda rec: (
  File "c:\python27\lib\site-packages\sqlalchemy\orm\persistence.py", line 485, in _collect_update_commands
    value, state.committed_state[propkey]) is not True:
  File "c:\python27\lib\site-packages\sqlalchemy\sql\type_api.py", line 359, in compare_values
    return x == y
TypeError: can't compare offset-naive and offset-aware datetimes

My data is in sql server, and some of the values look like this: image

The column type in sql server is datetime.

jfinkels commented 7 years ago

Hm this might be a bug in Flask-Restless, I'll look into it. Here is a failing test case that I will add to tests/test_updating.py:


    def test_updating_aware_datetime(self):
        # GitHub issue #630.                                                    
        now = datetime.now(tz=timezone.utc)
        person = self.Person(id=1, birth_datetime=now)
        self.session.add(person)
        self.session.commit()
        later = datetime.now(tz=timezone.utc)
        data = {
            'data': {
                'type': 'person',
                'id': '1',
                'attributes': {
                    'birth_datetime': later.isoformat()
                }
            }
        }
        response = self.app.patch('/api/person/1', data=dumps(data))
    print(response.data)
        self.assertEqual(response.status_code, 204)
        self.assertEqual(person.birth_datetime, later)
jfinkels commented 7 years ago

Actually, I believe this is a problem with the backend. Depending on your database, you may not have timezone support (or even datetime type support). I'm not sure what your database backend is, but you may try setting the timezone=True flag in the DateTime constructor; for more information, see http://docs.sqlalchemy.org/en/latest/core/type_basics.html#sqlalchemy.types.DateTime.

For example, in my unit tests, since I am using SQLite, even if I set timezone=True in the column, the following code demonstrates the problem:

        now = datetime.now(tz=timezone.utc)
        person = self.Person(id=1, birth_datetime=now)
        self.session.add(person)
        self.session.commit()

        print(now)
        # 2017-02-06 05:00:49.657781+00:00
        print(person.birth_datetime)
        # 2017-02-06 05:00:49.657781

So are you certain you have support for timezones in your database backend?

green3g commented 7 years ago

I'm using SQL Server, the datetime type, but I've also tried the datetime2 type. Neither one looks like it has time zone support, so shouldn't the default (timezone=False) work correctly?

jfinkels commented 7 years ago

What I'm saying is that your client is making a request to set a datetime field with a timezone-aware object; the "Z" in the datetime object in your original example indicates timezone-aware:

>>> from dateutil.parser import parse
>>> parse('2017-01-24T06:00:00.000Z')
datetime.datetime(2017, 1, 24, 6, 0, tzinfo=tzutc())
>>> parse('2017-01-24T06:00:00.000')
datetime.datetime(2017, 1, 24, 6, 0)

However, your database is timezone-naive, like in the second print statement in my example code above:

print(now)
# 2017-02-06 05:00:49.657781+00:00
print(person.birth_datetime)
# 2017-02-06 05:00:49.657781

So the request is a timezone-aware object, but the database is timezone-naive. There is a mismatch, and therefore SQLAlchemy raises an exception. None of this has anything to do with Flask-Restless except for the fact that we correctly parse your timezone-aware datetime from the string in the body of the request.

I will add a test that ensures a meaningful error message is sent to the client when this happens, but other than that, I'm going to mark this is as "not a bug".

jfinkels commented 7 years ago

Actually, the test I wrote in https://github.com/jfinkels/flask-restless/issues/630#issuecomment-277586698 doesn't demonstrate cause the TypeError that you initially reported. Perhaps it is particular to Microsoft SQL Server? @roemhildtg if you are able, can you provide a minimal working example that includes the Microsoft SQL Server-specific configuration code (since I don't use Microsoft Windows)?

green3g commented 7 years ago

I'll work on a minimal example.

Here's an update, I think this might have something to do with the issue. It also explains why I am so confused. One server PATCH requests are working correctly, the other is not, despite using the same code.

My overall workflow is the object is retrieved via GET, edited in a form, and then submitted back to flask restless via PATCH.

On the GET request, each server returns the object, but there is a discrepancy between the two in one of the date field, even though the data is the same.

On the PATCH request:

Could that have something to do with the issue?

green3g commented 7 years ago

Okay, here's a minimal example:

Created a table with id and date field:

image

import flask
import flask_sqlalchemy
import flask_restless

app = flask.Flask(__name__)
app.config['DEBUG'] = True
app.config['SQLALCHEMY_DATABASE_URI'] = 'mssql+pyodbc://gis'
db = flask_sqlalchemy.SQLAlchemy(app)

class DateTest(db.Model):
    __table_args__ = {'schema':'dbo'}
    __tablename__ = 'test_date_bug'
    id = db.Column(db.Integer, primary_key=True)
    some_date = db.Column(db.DateTime,)

manager = flask_restless.APIManager(app, flask_sqlalchemy_db=db)
manager.create_api(DateTest, methods=['GET', 'POST', 'DELETE', 'PATCH'])

app.run(host='0.0.0.0')
POST /api/test_date_bug

{"data":{"attributes":{"some_date":"2017-02-28T08:10:30.497Z"},"type":"test_date_bug"}}

RESULT

{
  "data": {
    "attributes": {
      "some_date": "2017-02-28T08:10:30.497000+00:00"
    },
    "id": "9",
    "links": {
      "self": "/api/test_date_bug/9"
    },
    "type": "test_date_bug"
  },
  "included": [],
  "jsonapi": {
    "version": "1.0"
  },
  "links": {},
  "meta": {}
}
GET /api/test_date_bug/9
{
  "data": {
    "attributes": {
      "some_date": "2017-02-28T08:10:30.497000"
    },
    "id": "9",
    "links": {
      "self": "/api/test_date_bug/9"
    },
    "type": "test_date_bug"
  },
  "included": [],
  "jsonapi": {
    "version": "1.0"
  },
  "links": {
    "self": "/api/test_date_bug/9"
  },
  "meta": {}
}
PATCH /api/test_date_bug/9

{"data":{"attributes":{"some_date":"2017-02-28T08:10:30.497000+00:00"},"type":"test_date_bug","id":"9"}}

Results in error. I think since the data is not timezone aware, flask restless should not return timezone aware dates in the POST and GET requests

jfinkels commented 7 years ago

Thanks for the example, I'll take a look at this.

jfinkels commented 7 years ago

Let's deal with each request individually.

  1. The POST request has a timezone-aware datetime attribute, and when the database row is created, the timezone information is silently dropped. This is not an issue with Flask-Restless, as that is an expected behavior of the database.
  2. The response to the POST request includes an incorrect representation of the created object. It is returning the timezone-aware datetime as requested by the client, but not the timezone-naive datetime that is actually created in the database. That is an issue and I have a fix for it that I will push.
  3. The GET request correctly returns the timezone-naive datetime, so I see no issue there.
  4. The PATCH request has a timezone-aware datetime attribute, and when the database row attempts to be updated, there is a potential conflict. On my system (64-bit Ubuntu 16.10, Python 3.5, SQLAlchemy 1.1.5 backed by SQLite 3.14.1) the timezone is silently dropped by the database and the row is correctly updated with the timezone-naive datetime. It seems that on your system, there seems to be a conflict.

What can we do about this (item 4)? I'm not sure. I don't want Flask-Restless to catch every possible type of error that might be raised (it's already catching essentially all SQLAlchemy exceptions). I'm not sure there's anything I can do about this. As a workaround, you can use a preprocessor to strip the timezones from the incoming requests.

As for your earlier comment about different versions of Python producing different versions of datetime strings, I'm guessing you are using print(d) in one place and print(d.isoformat()) in another:

$ python2 -c "from datetime import datetime; now = datetime.now(); print(now); print(now.isoformat())"
2017-03-25 00:08:04.802940
2017-03-25T00:08:04.802940

So again that is not a Flask-Restless issue.

jfinkels commented 7 years ago

I have created a pull request #647 to fix item 2 in my list above.

green3g commented 7 years ago

@jfinkels sounds good. Thanks for taking a look