[Freeipa-devel] [PATCH 0137] ipalib: Add DateTime parameter

[Petr Viktorin]

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.

-- 
Petr³