On 03/05/2014 01:08 PM, Jan Cholasta wrote: > On 25.2.2014 11:15, Tomas Babej wrote: >> >> 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. > > The first if statement is not necessary here, datetime values are > correctly handled in the super class: > > + def _convert_scalar(self, value, index=None): > + if isinstance(value, datetime.datetime): > + return value > + elif isinstance(value, basestring): > > > Please use ', ' instead of just ',' as the separator to make the error > message more readable here: > > + error = (_("does not match any of accepted formats: ") + > + (','.join(self.accepted_formats))) > > > This if statement is not present on old clients, so the assert below > will fail on them when they receive DateTime: > > + 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) > > But, they will never receive DateTime from us, because LDAP > generalized time is decoded to unicode in ipaldap. > > What I think we should do is decode LDAP generalized time to datetime > objects in ipaldap, add a new capability (e.g. "datetime_values") and > use DateTime only for clients with that capability, otherwise use the > generalized time string. Also, something similar should be done for > JSON, so that clients can infer that a value is datetime and not just > a generic string (e.g. wrap it in a dict with '__datetime__', similar > to how binary values are wrapped with '__base64__'), again only for > clients with the capability. >
Thank you for the suggestions. Updated, rebased patch is attached. -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org
>From d4e856e6a034556068f00db676558b83cbc21a4b Mon Sep 17 00:00:00 2001 From: Tomas Babej <[email protected]> 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 --- VERSION | 4 ++-- ipalib/__init__.py | 2 +- ipalib/capabilities.py | 3 +++ ipalib/constants.py | 2 ++ ipalib/parameters.py | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ ipalib/rpc.py | 27 ++++++++++++++++++++++++--- ipapython/ipaldap.py | 6 ++++++ 7 files changed, 88 insertions(+), 6 deletions(-) diff --git a/VERSION b/VERSION index 3b48d640787964afdc0769b7a6f490aaf3410548..32bddcf9dd0640e8b2710831fced0d6c1c3f23d8 100644 --- a/VERSION +++ b/VERSION @@ -89,5 +89,5 @@ IPA_DATA_VERSION=20100614120000 # # ######################################################## IPA_API_VERSION_MAJOR=2 -IPA_API_VERSION_MINOR=83 -# Last change: jcholast - add 'primary_key_types' capability +IPA_API_VERSION_MINOR=84 +# Last change: tbabej - added datetime value support 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/capabilities.py b/ipalib/capabilities.py index 3dd93294f430da88c8f4993a9cb96ce1202decbe..f2e45a0f66f596b23ed36c0e2c38ddc79e8625d7 100644 --- a/ipalib/capabilities.py +++ b/ipalib/capabilities.py @@ -48,6 +48,9 @@ capabilities = dict( # primary_key_types: Non-unicode primary keys in command output primary_key_types=u'2.83', + + # support for datetime values on the client + datetime_values=u'2.84' ) diff --git a/ipalib/constants.py b/ipalib/constants.py index 6cc50eacf44678840ad0048a1ef60c05736879cb..e98eee6f8bcf2c227daa88327326b4dd850fbfe0 100644 --- a/ipalib/constants.py +++ b/ipalib/constants.py @@ -206,3 +206,5 @@ DEFAULT_CONFIG = ( ('jsonrpc_uri', object), # derived from xmlrpc_uri in Env._finalize_core() ) + +LDAP_GENERALIZED_TIME_FORMAT = "%Y%m%d%H%M%SZ" diff --git a/ipalib/parameters.py b/ipalib/parameters.py index fc5e649815a71ea4a0f5f7a3f34ba9b7b846cfeb..fabe011530d78765c01bc806982d3f4cf2fe572f 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 @@ -1609,6 +1610,55 @@ 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, 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 73ae115b3e290f90803f11d8167d383f20844f5f..c44ffb6e19f452006d7dcf0a89f0644efb0aea01 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,17 +42,18 @@ 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 from nss.error import NSPRError from ipalib.backend import Connectible +from ipalib.constants import LDAP_GENERALIZED_TIME_FORMAT from ipalib.errors import (public_errors, UnknownError, NetworkError, KerberosError, XMLRPCMarshallError, JSONError, ConversionError) -from ipalib import errors +from ipalib import errors, capabilities from ipalib.request import context, Connection from ipalib.util import get_current_principal from ipapython.ipa_log_manager import root_logger @@ -163,6 +165,14 @@ def xml_wrap(value, version): return unicode(value) if isinstance(value, DN): return str(value) + + # Encode datetime.datetime objects as xmlrpclib.DateTime objects + if isinstance(value, datetime.datetime): + if capabilities.client_has_capability(version, 'datetime_values'): + return DateTime(value) + else: + return value.strftime(LDAP_GENERALIZED_TIME_FORMAT) + assert type(value) in (unicode, int, long, float, bool, NoneType) return value @@ -196,6 +206,9 @@ def xml_unwrap(value, encoding='UTF-8'): if isinstance(value, Binary): assert type(value.data) is str return value.data + if isinstance(value, DateTime): + # xmlprc DateTime is converted to string of %Y%m%dT%H:%M:%S format + return datetime.datetime.strptime(str(value), "%Y%m%dT%H:%M:%S") assert type(value) in (unicode, int, float, bool, NoneType) return value @@ -266,6 +279,11 @@ def json_encode_binary(val, version): return {'__base64__': base64.b64encode(str(val))} elif isinstance(val, DN): return str(val) + elif isinstance(val, datetime.datetime): + if capabilities.client_has_capability(version, 'datetime_values'): + return {'__datetime__': val.strftime(LDAP_GENERALIZED_TIME_FORMAT)} + else: + return val.strftime(LDAP_GENERALIZED_TIME_FORMAT) else: return val @@ -293,6 +311,9 @@ def json_decode_binary(val): if isinstance(val, dict): if '__base64__' in val: return base64.b64decode(val['__base64__']) + elif '__datetime__' in val: + return datetime.datetime.strptime(val['__datetime__'], + LDAP_GENERALIZED_TIME_FORMAT) else: return dict((k, json_decode_binary(v)) for k, v in val.items()) elif isinstance(val, list): diff --git a/ipapython/ipaldap.py b/ipapython/ipaldap.py index 56e7d13848b980329071d9f12309b5cbca5d5803..d939e759dfa0e870073ee330166482a4dda0f487 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 @@ -35,6 +36,7 @@ from ldap.controls import SimplePagedResultsControl import ldapurl from ipalib import errors, _ +from ipalib.constants import LDAP_GENERALIZED_TIME_FORMAT from ipapython import ipautil from ipapython.ipautil import ( format_netloc, wait_for_open_socket, wait_for_open_ports, CIDict) @@ -408,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(LDAP_GENERALIZED_TIME_FORMAT) elif val is None: return None else: @@ -439,6 +443,8 @@ class IPASimpleLDAPObject(object): elif isinstance(val, dict): dct = dict((unicode_from_utf8(k), self.decode(v, k)) for k, v in val.iteritems()) return dct + elif isinstance(val, datetime.datetime): + return val elif val is None: return None else: -- 1.8.5.3
_______________________________________________ Freeipa-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/freeipa-devel
