Merge lp:~openerp-dev/openobject-server/trunk-dates_py-niv into lp:openobject-server

[Nicolas Vanhoren (OpenERP)]

Added dates.py.

[lp:~openerp-dev/openobject-server/trunk-dates_py-niv]

4871. By Nicolas Vanhoren (OpenERP) on 2013-05-07

Fix problem with milliseconds coming from postgresql

[Vo Minh Thu (thu)]

Hi,

- Why all the `if not str: return str` and `if not obj: return False` ? What can they be when being falsy ? None, False, empty string ?

Actually I had a look at the code in the server and in the addons, and I'm not sure the case where we provide falsy values are common enough. At least, if I see `some_date.strptime()`, I know for sure that `some_date` can't be falsy (which is good).

- Same problem with the `.split('.')`. Again it seems a bit lousy.

- A comment such as "The datetime instance should not have an attached timezone and be in UTC." would be better complemented with an assertion.

- All the comments about "OpenERP 6.1 specification" or "Times in OpenERP are naive times." would be better in a single module-level docstring.

Overall I'm not sure we gain much with this helper functions:

- `time_to_str(obj)` is not much shorter than `obj.strftime(TIME_FORMAT)` and is more sloppy.
- With a quick grep, it seems str_to_date() would be of very little use...

I'm not against such helpers but I'm aginst having the core library used as a convenience library that doesn't even use its own helpers, or have an additional way to do things inconsistently:

We would have the DEFAULT_SERVER_*_FORMAT, *_FORMAT, the '%Y-%m-%d' literals, and your helpers.

Also, even by trying to replace occurences of DEFAULT_SERVER_*_FORMAT in the code, we could not remove them all (for instance we use them also with time instead of datetime).

Maybe we should try in theserver see if using your helpers really help.

review: Needs Fixing

[lp:~openerp-dev/openobject-server/trunk-dates_py-niv]

4872. By Nicolas Vanhoren (OpenERP) on 2013-05-13

changed the way we handle microseconds + removed xxx_to_str

[Nicolas Vanhoren (OpenERP) (niv-openerp)]

I took into account most of your critics.

Regarding the microseconds, to correctly explain the situation I need a long explanation:

First of all, the situation in PostgreSQL:

According to the documentation ( http://www.postgresql.org/docs/8.2/static/datatype-datetime.html ), Postgresql handles timestamps with a resolution up to 1 microsecond. Was is not very well explained is the way it handles the formatting of timestamps with microseconds. See this command line test:

testtrunk=# insert into all_readonly(datetime1) values ('2012-01-01 15:16:14');
INSERT 0 1
testtrunk=# select datetime1 from all_readonly where id = 1;
      datetime1
---------------------
 2012-01-01 15:16:14
(1 row)
testtrunk=# insert into all_readonly(datetime1) values ('2012-01-01 15:16:14.245');
INSERT 0 1
testtrunk=# select datetime1 from all_readonly where id = 1;
      datetime1
---------------------
 2012-01-01 15:16:14
(1 row)
testtrunk=# select datetime1 from all_readonly where id = 2;
        datetime1
-------------------------
 2012-01-01 15:16:14.245
(1 row)
testtrunk=# insert into all_readonly(datetime1) values ('2012-01-01 15:16:14.000');
INSERT 0 1
testtrunk=# select datetime1 from all_readonly where id = 3;
      datetime1
---------------------
 2012-01-01 15:16:14
(1 row)

By default, Postgresql accept timestamps that matches the regular expression "\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d(.\d+)?" (approximately). The microseconds are optional. When Postgresql format a timestamp, it skips the microseconds if they are equal to 0 (see last select).

Python's datetime module behavior:

It's probably a complete coincidence but Python's datetime.datetime.__str__ seems to format datetimes the same way Postgresql does. It only provides the microseconds in the output if that field is different to 0.

In [2]: from datetime import datetime

In [3]: x = datetime.now()

In [4]: str(x)
Out[4]: '2013-05-13 12:08:05.035149'

In [6]: y = x.replace(microsecond=0)

In [7]: str(y)
Out[7]: '2013-05-13 12:08:05'

Despite any logic, according to the Python documentation it seems impossible to correctly parse a datetime object converted to a string using using a single statement. strptime does not seem to provide a way to specify a field as optional. The shorter syntax that I can find is:

try:
    return datetime.datetime.strptime(str, "%Y-%m-%d %H:%M:%S")
except ValueError:
    return datetime.datetime.strptime(str, "%Y-%m-%d %H:%M:%S.%f")

OpenERP's behavior:

In [12]: connection = openerplib.get_connection(hostname="localhost", database="testtrunk", login="admin", password="admin")

In [13]: connection.get_model("all.readonly").write(1, {"datetime1": "2012-11-11 15:14:16"})
Out[13]: True

In [14]: connection.get_model("all.readonly").read(1, ["datetime1"])
Out[14]: {'datetime1': '2012-11-11 15:14:16', 'id': 1}

In [15]: connection.get_model("all.readonly").write(1, {"datetime1": "2012-11-11 15:14:16.213"})
Out[15]: True

In [16]: connection.get_model("all.readonly").read(1, ["datetime1"])
Out[16]: {'datetime1': '2012-11-11 15:14:16.213', 'id': 1}

In [17]: connection.get_model("all.readonly").write(1, {"datetime1": "2012-11-11 15:14:16.000"})
Out[17]: Tr...

[lp:~openerp-dev/openobject-server/trunk-dates_py-niv]

4873. By Nicolas Vanhoren (OpenERP) on 2013-05-13

typo

[Nicolas Vanhoren (OpenERP) (niv-openerp)]

Additional comment:

In the addons, there is 121 usages of datetime.strptime() to parse dates or datetimes. Approximately half of these concern datetimes. So, +- 60 pieces of codes in OpenERP are wrong because they not handle microseconds, which seems to be a valid value that the ORM can return. They do not crash most of the time because the web client has the bad habit to drop the microseconds in all datetimes it sends to the server (I think older clients did the same). But those codes will definitely crash if their value is provided by an external program using XML-RPC.

So, all usages of datetime.strptime should be replaced by str_to_datetime or str_to_date in the addons and the web client should be modified to stop dropping the microseconds.

review: Needs Resubmitting

Unmerged revisions

4873. By Nicolas Vanhoren (OpenERP) on 2013-05-13

typo

4872. By Nicolas Vanhoren (OpenERP) on 2013-05-13

changed the way we handle microseconds + removed xxx_to_str

4871. By Nicolas Vanhoren (OpenERP) on 2013-05-07

Fix problem with milliseconds coming from postgresql

4870. By Nicolas Vanhoren (OpenERP) on 2013-05-07

[IMP] added dates.py

4869. By Nicolas Vanhoren (OpenERP) on 2013-05-07

merge trunk

=== added file 'openerp/tools/dates.py'
--- openerp/tools/dates.py	1970-01-01 00:00:00 +0000
+++ openerp/tools/dates.py	2013-05-13 10:30:33 +0000
@@ -0,0 +1,43 @@
+# -*- coding: utf-8 -*-
+##############################################################################
+#
+#    OpenERP, Open Source Management Solution
+#    Copyright (C) 2013 OpenERP s.a. (<http://openerp.com>).
+#
+#    This program is free software: you can redistribute it and/or modify
+#    it under the terms of the GNU Affero General Public License as
+#    published by the Free Software Foundation, either version 3 of the
+#    License, or (at your option) any later version.
+#
+#    This program is distributed in the hope that it will be useful,
+#    but WITHOUT ANY WARRANTY; without even the implied warranty of
+#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#    GNU Affero General Public License for more details.
+#
+#    You should have received a copy of the GNU Affero General Public License
+#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+##############################################################################
+
+import datetime
+import openerp.tools.misc as misc
+
+def str_to_datetime(str):
+    """
+    Converts a string to a datetime object using OpenERP's
+    datetime string format (example: '2011-12-01 15:12:35' or '2011-12-01 15:12:35.535').
+    
+    No timezone information is added, the datetime is a naive instance, but
+    according to OpenERP 6.1 specification the timezone is always UTC.
+    """
+    try:
+        return datetime.datetime.strptime(str, "%Y-%m-%d %H:%M:%S")
+    except ValueError:
+        return datetime.datetime.strptime(str, "%Y-%m-%d %H:%M:%S.%f")
+
+def str_to_date(str):
+    """
+    Converts a string to a date object using OpenERP's
+    date string format (exemple: '2011-12-01').
+    """
+    return datetime.datetime.strptime(str, "%Y-%m-%d").date()