On 01/14/2014 10:19 AM, Petr Viktorin wrote:
> On 01/14/2014 09:27 AM, Jan Cholasta wrote:
>> On 13.1.2014 14:57, Petr Vobornik wrote:
>>> On 13.1.2014 13:41, Jan Cholasta wrote:
>>>> Hi,
>>>>
>>>> On 10.1.2014 21:21, Nathaniel McCallum wrote:
>>>>> On Thu, 2014-01-09 at 16:30 +0100, Tomas Babej wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Adds a parameter that represents a DateTime format using
>>>>>> datetime.datetime
>>>>>> object from python's native datetime library.
>>>>>>
>>>>>> In the CLI, accepts one of the following formats:
>>>>>> Accepts subset of values defined by ISO 8601:
>>>>>> %Y-%m-%dT%H:%M:%S
>>>>>> %Y-%m-%dT%H:%M
>>>>>> '%Y%m%dT%H:%M:%S'
>>>>>> '%Y%m%dT%H:%M'
>>>>>>
>>>>>> Also accepts LDAP Generalized time in the following format:
>>>>>> '%Y%m%d%H%M%SZ'
>>>>>>
>>>>>> As a simplification, it does not deal with timezone info and ISO
>>>>>> 8601
>>>>>> values with timezone info (+-hhmm) are rejected. Values are expected
>>>>>> to be in the UTC timezone.
>>>>>>
>>>>>> Values are saved to LDAP as LDAP Generalized time values in the
>>>>>> format
>>>>>> '%Y%m%d%H%SZ' (no time fractions and UTC timezone is assumed). To
>>>>>> avoid
>>>>>> confusion, in addition to subset of ISO 8601 values, the LDAP
>>>>>> generalized
>>>>>> time in the format '%Y%m%d%H%M%SZ' is also accepted as an input (as
>>>>>> this
>>>>>> is the
>>>>>> format user will see on the output).
>>>>>>
>>>>>> Part of: https://fedorahosted.org/freeipa/ticket/3306
>>>>>
>>>>> The date/time syntax formats are not compliant with ISO 8601. You
>>>>> stated
>>>>> they are expected to be in UTC timezone, but no 'Z' is expected in
>>>>> most
>>>>> of them. This is not only non-standard, but would prevent you from
>>>>> every
>>>>> supporting local time in the future.
>>>>
>>>> +1, please require the trailing "Z".
>>>>
>>>>
>>>> I think generalized time format should be the preferred one, as it is
>>>> stored in LDAP and thus returned by commands. Also, do we really need
>>>> all of these other formats?
>>>
>>> +1 That API should accept and always return generalized time.
>>>
>>> But UIs(CLI, Web) should display something more human readable. Like:
>>> %Y-%m-%dT%H:%M:%SZ or IMHO better (but not ISO 8601): %Y-%m-%d
>>> %H:%M:%SZ
>>> ie. 2014-03-08 14:16:55Z compared to 2014-03-08T14:16:55Z or
>>> 20140308141655Z
>>>
>>> Both should accept input value in the same format. Usage of different
>>> output and input formats is confusing. Thus I agree with supporting
>>> more
>>> input formats. Decision whether it should be done on API level or
>>> client
>>> level is another matter.
>>
>> If you want human readable output, you have to do the conversion from
>> generalized time on clients. If you want to support local time and
>> timezones, you have to do some processing on the client as well. I would
>> be perfectly happy if we supported only generalized time over the wire
>> and did conversion from/to human readable on clients.
>>
>>>
>>> I has been implementing the Web UI part and I think we should also
>>> support a formats without time component. My initial impl. supports
>>> combinations of:
>>>
>>>      var dates = [
>>>          ['YYYY-MM-DD', /^(\d{4})-(\d{2})-(\d{2})$/],
>>>          ['YYYYMMDD',/^(\d{4})(\d{2})(\d{2})$/]
>>>      ];
>>>
>>>      var times = [
>>>          ['HH:mm:ss', /^(\d\d):(\d\d):(\d\d)Z$/],
>>>          ['HH:mm', /^(\d\d):(\d\d)Z$/],
>>>          ['HH', /^(\d\d)Z$/]
>>>      ];
>>> + generalized time
>>> ['YYYYMMDDHHmmssZ',/^(\d{4})(\d{2})(\d{2})(\d{2})(\d{2})(\d{2})Z$/]
>>>
>>> with /( |T)/ as divider and time optional.
>>>
>>> Both UIs should also tell the user what is the expected format (at
>>> least
>>> one of them). Getting incorrect format error without knowing it is
>>> annoying.
>>
>> The current code raises a ValidationError including the list of formats.
>
> On yesterday's meeting, Simo said that on the wire and for CLI output,
> we should use generalized time.
> I disagree with using it for CLI ouptut, as I find generalized time
> unreadable, but for the API it's the obvious choice.
>
> I believe we should require the "Z" postfix in all formats, so that 1)
> users are forced to be aware that it's UTC, and 2) we can
> theoretically support local time in the future.
>
> I think the supported input formats should be:
>
> %Y%m%d%H%M%SZ      (generalized time)
> %Y-%m-%dT%H:%M:%SZ (ISO 8601, with seconds)
> %Y-%m-%dT%H:%MZ    (ISO 8601, without seconds)
> %Y-%m-%dZ          (ISO 8601, date only)
>
> I also think we should accept a space instead of the "T", as PetrĀ²
> said above.
>

Thanks for review, everybody.

* All accepted formats now require explicit Z
* Accept values with ' ' instead of T
* LDAP generalized time used on the wire with JSON
* Minor implementation fixes

Updated patch should address all the issues.

-- 
Tomas Babej
Associate Software Engeneer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org 

>From 4153e0010e5cf9eb5565e9bb2e29ec0e21c43e5d Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Thu, 9 Jan 2014 11:14:56 +0100
Subject: [PATCH] ipalib: Add DateTime parameter

Adds a parameter that represents a DateTime format using datetime.datetime
object from python's native datetime library.

In the CLI, accepts one of the following formats:
    Accepts LDAP Generalized time without in the following format:
       '%Y%m%d%H%M%SZ'

    Accepts subset of values defined by ISO 8601:
        '%Y-%m-%dT%H:%M:%SZ'
        '%Y-%m-%dT%H:%MZ'
        '%Y-%m-%dZ'

    Also accepts above formats using ' ' (space) as a separator instead of 'T'.

As a simplification, it does not deal with timezone info and ISO 8601
values with timezone info (+-hhmm) are rejected. Values are expected
to be in the UTC timezone.

Values are saved to LDAP as LDAP Generalized time values in the format
'%Y%m%d%H%SZ' (no time fractions and UTC timezone is assumed). To avoid
confusion, in addition to subset of ISO 8601 values, the LDAP generalized
time in the format '%Y%m%d%H%M%SZ' is also accepted as an input (as this is the
format user will see on the output).

Part of: https://fedorahosted.org/freeipa/ticket/3306
---
 ipalib/__init__.py   |  2 +-
 ipalib/parameters.py | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 ipalib/rpc.py        | 14 ++++++++++++--
 ipapython/ipaldap.py |  3 +++
 4 files changed, 68 insertions(+), 3 deletions(-)

diff --git a/ipalib/__init__.py b/ipalib/__init__.py
index 553c07197ddc95025da9df9e6c1fcddadadff4d8..2a87103b8dcb03a6d890029b830195cde52fc1e6 100644
--- a/ipalib/__init__.py
+++ b/ipalib/__init__.py
@@ -886,7 +886,7 @@ from frontend import Command, LocalOrRemote, Updater, Advice
 from frontend import Object, Method
 from crud import Create, Retrieve, Update, Delete, Search
 from parameters import DefaultFrom, Bool, Flag, Int, Decimal, Bytes, Str, IA5Str, Password, DNParam, DeprecatedParam
-from parameters import BytesEnum, StrEnum, IntEnum, AccessTime, File
+from parameters import BytesEnum, StrEnum, IntEnum, AccessTime, File, DateTime
 from errors import SkipPluginModule
 from text import _, ngettext, GettextFactory, NGettextFactory
 
diff --git a/ipalib/parameters.py b/ipalib/parameters.py
index b4fb3402df0ab1af8fb71086ccf22ee3a704b322..60066cf42c34a9e6c2a66467ed85a412c386bc5f 100644
--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -102,6 +102,7 @@ a more detailed description for clarity.
 import re
 import decimal
 import base64
+import datetime
 from xmlrpclib import MAXINT, MININT
 
 from types import NoneType
@@ -1604,6 +1605,57 @@ class File(Str):
         ('noextrawhitespace', bool, False),
     )
 
+class DateTime(Param):
+    """
+    DateTime parameter type.
+
+    Accepts LDAP Generalized time without in the following format:
+       '%Y%m%d%H%M%SZ'
+
+    Accepts subset of values defined by ISO 8601:
+        '%Y-%m-%dT%H:%M:%SZ'
+        '%Y-%m-%dT%H:%MZ'
+        '%Y-%m-%dZ'
+
+    Also accepts above formats using ' ' (space) as a separator instead of 'T'.
+
+    Refer to the `man strftime` for the explanations for the %Y,%m,%d,%H.%M,%S.
+    """
+
+    accepted_formats = ['%Y%m%d%H%M%SZ',       # generalized time
+                        '%Y-%m-%dT%H:%M:%SZ',  # ISO 8601, second precision
+                        '%Y-%m-%dT%H:%MZ',     # ISO 8601, minute precision
+                        '%Y-%m-%dZ',           # ISO 8601, date only
+                        '%Y-%m-%d %H:%M:%SZ',  # non-ISO 8601, second precision
+                        '%Y-%m-%d %H:%MZ']     # non-ISO 8601, minute precision
+
+
+    type = datetime.datetime
+    type_error = _('must be datetime value')
+
+    def _convert_scalar(self, value, index=None):
+        if isinstance(value, datetime.datetime):
+            return value
+        elif isinstance(value, basestring):
+            for date_format in self.accepted_formats:
+                try:
+                    time = datetime.datetime.strptime(value, date_format)
+                    return time
+                except ValueError:
+                    pass
+
+            # If we get here, the strptime call did not succeed for any
+            # the accepted formats, therefore raise error
+
+            error = (_("does not match any of accepted formats: ") +
+                      (','.join(self.accepted_formats)))
+
+            raise ConversionError(name=self.get_param_name(),
+                                  index=index,
+                                  error=error)
+
+        return super(DateTime, self)._convert_scalar(value, index)
+
 
 class AccessTime(Str):
     """
diff --git a/ipalib/rpc.py b/ipalib/rpc.py
index 2b47d1c0e25bbeec0dde38089f444e0399e1670e..476143c497517437882fd287dda74b296598b17f 100644
--- a/ipalib/rpc.py
+++ b/ipalib/rpc.py
@@ -33,6 +33,7 @@ Also see the `ipaserver.rpcserver` module.
 from types import NoneType
 from decimal import Decimal
 import sys
+import datetime
 import os
 import locale
 import base64
@@ -41,8 +42,8 @@ import json
 import socket
 from urllib2 import urlparse
 
-from xmlrpclib import (Binary, Fault, dumps, loads, ServerProxy, Transport,
-        ProtocolError, MININT, MAXINT)
+from xmlrpclib import (Binary, Fault, DateTime, dumps, loads, ServerProxy,
+        Transport, ProtocolError, MININT, MAXINT)
 import kerberos
 from dns import resolver, rdatatype
 from dns.exception import DNSException
@@ -162,6 +163,11 @@ def xml_wrap(value):
         return unicode(value)
     if isinstance(value, DN):
         return str(value)
+
+    # Encode datetime.datetime objects as xmlrpclib.DateTime objects
+    if isinstance(value, datetime.datetime):
+        return DateTime(value)
+
     assert type(value) in (unicode, int, long, float, bool, NoneType)
     return value
 
@@ -195,6 +201,8 @@ def xml_unwrap(value, encoding='UTF-8'):
     if isinstance(value, Binary):
         assert type(value.data) is str
         return value.data
+    if isinstance(value, DateTime):
+        return datetime.datetime.strptime(str(value), "%Y%m%dT%H:%M:%S")
     assert type(value) in (unicode, int, float, bool, NoneType)
     return value
 
@@ -264,6 +272,8 @@ def json_encode_binary(val):
         return {'__base64__': base64.b64encode(str(val))}
     elif isinstance(val, DN):
         return str(val)
+    elif isinstance(val, datetime.datetime):
+        return val.strftime("%Y%m%d%H%M%SZ")
     else:
         return val
 
diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py
index 074e0c21935f3cf343f14faa4f891e75124dd7f8..3562ded7fd99fcef7eb3815049879c112821e395 100644
--- a/ipapython/ipaldap.py
+++ b/ipapython/ipaldap.py
@@ -21,6 +21,7 @@
 
 import string
 import time
+import datetime
 import shutil
 from decimal import Decimal
 from copy import deepcopy
@@ -409,6 +410,8 @@ class IPASimpleLDAPObject(object):
         elif isinstance(val, dict):
             dct = dict((self.encode(k), self.encode(v)) for k, v in val.iteritems())
             return dct
+        elif isinstance(val, datetime.datetime):
+            return val.strftime("%Y%m%d%H%M%SZ")
         elif val is None:
             return None
         else:
-- 
1.8.5.3

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to