Re: [Freeipa-devel] [PATCH 0137] ipalib: Add DateTime parameter
On 05/07/2014 06:15 PM, Dmitri Pal wrote: On 05/07/2014 11:46 AM, Nathaniel McCallum wrote: On Wed, 2014-05-07 at 09:50 -0400, Dmitri Pal wrote: On 05/07/2014 04:06 AM, Jan Cholasta wrote: On 6.5.2014 19:55, Nathaniel McCallum wrote: I know it is a bit late on this, but for the OTP token import script, I have to have support for the full ISO 8601. My plan right now is to use python-dateutil for this. Using dateutil would simplify some of this code. Is there a reason we aren't using dateutil? IIRC it was rejected right at the beginning as an overkill. AFAICS python-dateutil allows a lot of formats other than ISO 8601, including 5:50 A.M. on June 13, 1990 or even 09-25-2003. I'd rather stick to something well-defined, and fail hard on potentially ambiguous formats. I dont' see a guarantee that python-dateutil won't change the ad-hoc formats in the future. What are the alternatives? Hand-coded date parsing, AFAICS. That is what we are currently doing. In order to make this sane, we greatly restrict the date formats permitted as input to a small subset of ISO 8601. This is possible because we just tell the users to type the date in one of the supported formats. One difference is that we allow using a space instead of 'T' for datetime values. The standard allows omitting the T by mutual agreement, and python-dateutil does support this. Unfortunately, I can't make that trade-off in the token import script since I have no control over the input. Since I have to support all of ISO 8601 (including timezone conversions), the dateutils module is pretty much the only option. If we adopt it for OTP import, we might as well throw away our hand-coded date parsing. I'm not a fan of the full ISO 8601 beast (sections 5.3-5.5 of RFC3339 sum it up nicely), but it's a standard; if we can convince python-dateutil to only parse ISO 8601 and generalized time, let's use it. Nathaniel I would leave this till next week for Martin and Petr3 to chime in. I do not have a problem but I am not sure I can assess all the implications. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0137] ipalib: Add DateTime parameter
On 05/13/2014 12:54 PM, Petr Viktorin wrote: On 05/07/2014 06:15 PM, Dmitri Pal wrote: On 05/07/2014 11:46 AM, Nathaniel McCallum wrote: On Wed, 2014-05-07 at 09:50 -0400, Dmitri Pal wrote: On 05/07/2014 04:06 AM, Jan Cholasta wrote: On 6.5.2014 19:55, Nathaniel McCallum wrote: I know it is a bit late on this, but for the OTP token import script, I have to have support for the full ISO 8601. My plan right now is to use python-dateutil for this. Using dateutil would simplify some of this code. Is there a reason we aren't using dateutil? IIRC it was rejected right at the beginning as an overkill. AFAICS python-dateutil allows a lot of formats other than ISO 8601, including 5:50 A.M. on June 13, 1990 or even 09-25-2003. I'd rather stick to something well-defined, and fail hard on potentially ambiguous formats. I dont' see a guarantee that python-dateutil won't change the ad-hoc formats in the future. What are the alternatives? Hand-coded date parsing, AFAICS. That is what we are currently doing. In order to make this sane, we greatly restrict the date formats permitted as input to a small subset of ISO 8601. This is possible because we just tell the users to type the date in one of the supported formats. One difference is that we allow using a space instead of 'T' for datetime values. The standard allows omitting the T by mutual agreement, and python-dateutil does support this. Unfortunately, I can't make that trade-off in the token import script since I have no control over the input. Since I have to support all of ISO 8601 (including timezone conversions), the dateutils module is pretty much the only option. If we adopt it for OTP import, we might as well throw away our hand-coded date parsing. I'm not a fan of the full ISO 8601 beast (sections 5.3-5.5 of RFC3339 sum it up nicely), but it's a standard; if we can convince python-dateutil to only parse ISO 8601 and generalized time, let's use it. Let me drop one heretic idea - what about having 2 parameters? GeneralDateTime with the general parsing (via dateutil) for OTP specific use case and then SimplifiedDateTime parameter for plugins that needs just basic date time? I.e. with manual parsing that Tomas already implemented and that was acked on our devel meeting? Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0137] ipalib: Add DateTime parameter
On 13.5.2014 13:24, Martin Kosek wrote: On 05/13/2014 12:54 PM, Petr Viktorin wrote: On 05/07/2014 06:15 PM, Dmitri Pal wrote: On 05/07/2014 11:46 AM, Nathaniel McCallum wrote: On Wed, 2014-05-07 at 09:50 -0400, Dmitri Pal wrote: On 05/07/2014 04:06 AM, Jan Cholasta wrote: On 6.5.2014 19:55, Nathaniel McCallum wrote: I know it is a bit late on this, but for the OTP token import script, I have to have support for the full ISO 8601. My plan right now is to use python-dateutil for this. Using dateutil would simplify some of this code. Is there a reason we aren't using dateutil? IIRC it was rejected right at the beginning as an overkill. AFAICS python-dateutil allows a lot of formats other than ISO 8601, including 5:50 A.M. on June 13, 1990 or even 09-25-2003. I'd rather stick to something well-defined, and fail hard on potentially ambiguous formats. I dont' see a guarantee that python-dateutil won't change the ad-hoc formats in the future. What are the alternatives? Hand-coded date parsing, AFAICS. That is what we are currently doing. In order to make this sane, we greatly restrict the date formats permitted as input to a small subset of ISO 8601. This is possible because we just tell the users to type the date in one of the supported formats. One difference is that we allow using a space instead of 'T' for datetime values. The standard allows omitting the T by mutual agreement, and python-dateutil does support this. Unfortunately, I can't make that trade-off in the token import script since I have no control over the input. Since I have to support all of ISO 8601 (including timezone conversions), the dateutils module is pretty much the only option. If we adopt it for OTP import, we might as well throw away our hand-coded date parsing. I'm not a fan of the full ISO 8601 beast (sections 5.3-5.5 of RFC3339 sum it up nicely), but it's a standard; if we can convince python-dateutil to only parse ISO 8601 and generalized time, let's use it. Let me drop one heretic idea - what about having 2 parameters? GeneralDateTime with the general parsing (via dateutil) for OTP specific use case and then SimplifiedDateTime parameter for plugins that needs just basic date time? I.e. with manual parsing that Tomas already implemented and that was acked on our devel meeting? I don't see why any of this should be supported at the API level. If you need to do some preprocessing, fine, but do it on the client side. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0137] ipalib: Add DateTime parameter
On 6.5.2014 19:55, Nathaniel McCallum wrote: I know it is a bit late on this, but for the OTP token import script, I have to have support for the full ISO 8601. My plan right now is to use python-dateutil for this. Using dateutil would simplify some of this code. Is there a reason we aren't using dateutil? IIRC it was rejected right at the beginning as an overkill. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0137] ipalib: Add DateTime parameter
On 05/07/2014 04:06 AM, Jan Cholasta wrote: On 6.5.2014 19:55, Nathaniel McCallum wrote: I know it is a bit late on this, but for the OTP token import script, I have to have support for the full ISO 8601. My plan right now is to use python-dateutil for this. Using dateutil would simplify some of this code. Is there a reason we aren't using dateutil? IIRC it was rejected right at the beginning as an overkill. What are the alternatives? -- Thank you, Dmitri Pal Sr. Engineering Manager IdM portfolio Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0137] ipalib: Add DateTime parameter
On Wed, 2014-05-07 at 09:50 -0400, Dmitri Pal wrote: On 05/07/2014 04:06 AM, Jan Cholasta wrote: On 6.5.2014 19:55, Nathaniel McCallum wrote: I know it is a bit late on this, but for the OTP token import script, I have to have support for the full ISO 8601. My plan right now is to use python-dateutil for this. Using dateutil would simplify some of this code. Is there a reason we aren't using dateutil? IIRC it was rejected right at the beginning as an overkill. What are the alternatives? Hand-coded date parsing, AFAICS. That is what we are currently doing. In order to make this sane, we greatly restrict the date formats permitted as input to a small subset of ISO 8601. This is possible because we just tell the users to type the date in one of the supported formats. Unfortunately, I can't make that trade-off in the token import script since I have no control over the input. Since I have to support all of ISO 8601 (including timezone conversions), the dateutils module is pretty much the only option. If we adopt it for OTP import, we might as well throw away our hand-coded date parsing. Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0137] ipalib: Add DateTime parameter
On 05/07/2014 11:46 AM, Nathaniel McCallum wrote: On Wed, 2014-05-07 at 09:50 -0400, Dmitri Pal wrote: On 05/07/2014 04:06 AM, Jan Cholasta wrote: On 6.5.2014 19:55, Nathaniel McCallum wrote: I know it is a bit late on this, but for the OTP token import script, I have to have support for the full ISO 8601. My plan right now is to use python-dateutil for this. Using dateutil would simplify some of this code. Is there a reason we aren't using dateutil? IIRC it was rejected right at the beginning as an overkill. What are the alternatives? Hand-coded date parsing, AFAICS. That is what we are currently doing. In order to make this sane, we greatly restrict the date formats permitted as input to a small subset of ISO 8601. This is possible because we just tell the users to type the date in one of the supported formats. Unfortunately, I can't make that trade-off in the token import script since I have no control over the input. Since I have to support all of ISO 8601 (including timezone conversions), the dateutils module is pretty much the only option. If we adopt it for OTP import, we might as well throw away our hand-coded date parsing. Nathaniel I would leave this till next week for Martin and Petr3 to chime in. I do not have a problem but I am not sure I can assess all the implications. -- Thank you, Dmitri Pal Sr. Engineering Manager IdM portfolio Red Hat, Inc. ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0137] ipalib: Add DateTime parameter
I know it is a bit late on this, but for the OTP token import script, I
have to have support for the full ISO 8601. My plan right now is to use
python-dateutil for this.
Using dateutil would simplify some of this code. Is there a reason we
aren't using dateutil?
On Mon, 2014-05-05 at 18:58 +0300, Alexander Bokovoy wrote:
On Wed, 30 Apr 2014, Tomas Babej wrote:
On 04/25/2014 11:08 AM, Jan Cholasta wrote:
On 22.4.2014 13:32, Tomas Babej wrote:
Thank you for the suggestions. Updated, rebased patch is attached.
This API.txt change from the next patch belongs in this patch:
+capability: datetime_values 2.84
I think you should use the LDAP_GENERALIZED_TIME_FORMAT constant here:
+accepted_formats = ['%Y%m%d%H%M%SZ', # generalized time
This is not right:
+elif isinstance(val, datetime.datetime):
+return val
To actually decode LDAP generalized time attributes to datetime, you
need to do this:
'2.16.840.1.113719.1.301.4.41.1' : DN, # krbSubTrees
'2.16.840.1.113719.1.301.4.52.1' : DN, # krbObjectReferences
'2.16.840.1.113719.1.301.4.53.1' : DN, # krbPrincContainerRef
+
+'1.3.6.1.4.1.1466.115.121.1.24' : datetime.datetime,
}
# In most cases we lookup the syntax from the schema returned by
and this:
return val
elif target_type is unicode:
return val.decode('utf-8')
+elif target_type is datetime.datetime:
+return datetime.datetime.strptime(val,
LDAP_GENERALIZED_TIME_FORMAT)
else:
return target_type(val)
except Exception, e:
and add code for formatting datetime values to the textui backend.
Thanks for the review. I fixed all the issues, updated patch is attached.
I also added unit tests for the new DateTime parameter.
Thanks, tested them as part of kerberos principal expiration time
patches.
Pushed two patches to git master.
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0137] ipalib: Add DateTime parameter
On Wed, 30 Apr 2014, Tomas Babej wrote:
On 04/25/2014 11:08 AM, Jan Cholasta wrote:
On 22.4.2014 13:32, Tomas Babej wrote:
Thank you for the suggestions. Updated, rebased patch is attached.
This API.txt change from the next patch belongs in this patch:
+capability: datetime_values 2.84
I think you should use the LDAP_GENERALIZED_TIME_FORMAT constant here:
+accepted_formats = ['%Y%m%d%H%M%SZ', # generalized time
This is not right:
+elif isinstance(val, datetime.datetime):
+return val
To actually decode LDAP generalized time attributes to datetime, you
need to do this:
'2.16.840.1.113719.1.301.4.41.1' : DN, # krbSubTrees
'2.16.840.1.113719.1.301.4.52.1' : DN, # krbObjectReferences
'2.16.840.1.113719.1.301.4.53.1' : DN, # krbPrincContainerRef
+
+'1.3.6.1.4.1.1466.115.121.1.24' : datetime.datetime,
}
# In most cases we lookup the syntax from the schema returned by
and this:
return val
elif target_type is unicode:
return val.decode('utf-8')
+elif target_type is datetime.datetime:
+return datetime.datetime.strptime(val,
LDAP_GENERALIZED_TIME_FORMAT)
else:
return target_type(val)
except Exception, e:
and add code for formatting datetime values to the textui backend.
Thanks for the review. I fixed all the issues, updated patch is attached.
I also added unit tests for the new DateTime parameter.
Thanks, tested them as part of kerberos principal expiration time
patches.
Pushed two patches to git master.
--
/ Alexander Bokovoy
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0137] ipalib: Add DateTime parameter
On 04/25/2014 11:08 AM, Jan Cholasta wrote:
On 22.4.2014 13:32, Tomas Babej wrote:
Thank you for the suggestions. Updated, rebased patch is attached.
This API.txt change from the next patch belongs in this patch:
+capability: datetime_values 2.84
I think you should use the LDAP_GENERALIZED_TIME_FORMAT constant here:
+accepted_formats = ['%Y%m%d%H%M%SZ', # generalized time
This is not right:
+elif isinstance(val, datetime.datetime):
+return val
To actually decode LDAP generalized time attributes to datetime, you
need to do this:
'2.16.840.1.113719.1.301.4.41.1' : DN, # krbSubTrees
'2.16.840.1.113719.1.301.4.52.1' : DN, # krbObjectReferences
'2.16.840.1.113719.1.301.4.53.1' : DN, # krbPrincContainerRef
+
+'1.3.6.1.4.1.1466.115.121.1.24' : datetime.datetime,
}
# In most cases we lookup the syntax from the schema returned by
and this:
return val
elif target_type is unicode:
return val.decode('utf-8')
+elif target_type is datetime.datetime:
+return datetime.datetime.strptime(val,
LDAP_GENERALIZED_TIME_FORMAT)
else:
return target_type(val)
except Exception, e:
and add code for formatting datetime values to the textui backend.
Thanks for the review. I fixed all the issues, updated patch is attached.
I also added unit tests for the new DateTime parameter.
--
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org
From d5f36d18dbc29fc4e8f01b84a4e786a1e79dacc4 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
---
API.txt| 1 +
VERSION| 4 ++--
ipalib/__init__.py | 2 +-
ipalib/capabilities.py | 3 +++
ipalib/cli.py | 6 +-
ipalib/constants.py| 2 ++
ipalib/parameters.py | 52 +-
ipalib/rpc.py | 27 +++---
ipapython/ipaldap.py | 7 +++
9 files changed, 96 insertions(+), 8 deletions(-)
diff --git a/API.txt b/API.txt
index c2654b144fadbdd789308f9d01fa33ccdd9b7960..eb8a61f645235610eba8dc6d3452a835a7af8b1a 100644
--- a/API.txt
+++ b/API.txt
@@ -4007,3 +4007,4 @@ capability: messages 2.52
capability: optional_uid_params 2.54
capability: permissions2 2.69
capability: primary_key_types 2.83
+capability: datetime_values 2.84
diff --git a/VERSION b/VERSION
index 3b48d640787964afdc0769b7a6f490aaf3410548..32bddcf9dd0640e8b2710831fced0d6c1c3f23d8 100644
--- a/VERSION
+++ b/VERSION
@@ -89,5 +89,5 @@ IPA_DATA_VERSION=2010061412
# #
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
Re: [Freeipa-devel] [PATCH 0137] ipalib: Add DateTime parameter
On 22.4.2014 13:32, Tomas Babej wrote:
Thank you for the suggestions. Updated, rebased patch is attached.
This API.txt change from the next patch belongs in this patch:
+capability: datetime_values 2.84
I think you should use the LDAP_GENERALIZED_TIME_FORMAT constant here:
+accepted_formats = ['%Y%m%d%H%M%SZ', # generalized time
This is not right:
+elif isinstance(val, datetime.datetime):
+return val
To actually decode LDAP generalized time attributes to datetime, you
need to do this:
'2.16.840.1.113719.1.301.4.41.1' : DN, # krbSubTrees
'2.16.840.1.113719.1.301.4.52.1' : DN, # krbObjectReferences
'2.16.840.1.113719.1.301.4.53.1' : DN, # krbPrincContainerRef
+
+'1.3.6.1.4.1.1466.115.121.1.24' : datetime.datetime,
}
# In most cases we lookup the syntax from the schema returned by
and this:
return val
elif target_type is unicode:
return val.decode('utf-8')
+elif target_type is datetime.datetime:
+return datetime.datetime.strptime(val,
LDAP_GENERALIZED_TIME_FORMAT)
else:
return target_type(val)
except Exception, e:
and add code for formatting datetime values to the textui backend.
--
Jan Cholasta
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0137] ipalib: Add DateTime parameter
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 = [
['-MM-DD', /^(\d{4})-(\d{2})-(\d{2})$/],
['MMDD',/^(\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
['MMDDHHmmssZ',/^(\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
Re: [Freeipa-devel] [PATCH 0137] ipalib: Add DateTime parameter
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 = [
['-MM-DD', /^(\d{4})-(\d{2})-(\d{2})$/],
['MMDD',/^(\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
['MMDDHHmmssZ',/^(\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
Re: [Freeipa-devel] [PATCH 0137] ipalib: Add DateTime parameter
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 = [
['-MM-DD', /^(\d{4})-(\d{2})-(\d{2})$/],
['MMDD',/^(\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
['MMDDHHmmssZ',/^(\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:
Re: [Freeipa-devel] [PATCH 0137] ipalib: Add DateTime parameter
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 = [
['-MM-DD', /^(\d{4})-(\d{2})-(\d{2})$/],
['MMDD',/^(\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
['MMDDHHmmssZ',/^(\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.
Few nitpicks about DateTime implementation:
* you should call Param._convert_scalar from DateTime._convert_scalar
* raise ConversionError instead of ValidationError and use
get_param_name() for name
* instead of isinstance(value, str) or isinstance(value, unicode)
use isinstance(value, basestring)
* don't use pythonisms in public error messages (does not match any
of accepted formats: %s % self.accepted_formats)
* public error messages should be translatable
It should look something like this (untested, from top of my head):
type = datetime.datetime
type_error = _('must be a date/time 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: %s) %
(', '.join(self.accepted_formats))
raise ConversionError(name=self.get_param_name(),
index=index,
error=error)
return super(DateTime, self)._convert_scalar(value, index)
+if isinstance(value, DateTime):
+return str(value)
Return datetime object here please, or at least unicode in generalized
time format (or whatever the preferred format is, see above).
+elif isinstance(val, datetime.datetime):
+return val.strftime(%Y%m%dT%H:%M:%S)
Again, please use the
Re: [Freeipa-devel] [PATCH 0137] ipalib: Add DateTime parameter
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 = [
['-MM-DD', /^(\d{4})-(\d{2})-(\d{2})$/],
['MMDD',/^(\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
['MMDDHHmmssZ',/^(\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³
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0137] ipalib: Add DateTime parameter
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?
Few nitpicks about DateTime implementation:
* you should call Param._convert_scalar from DateTime._convert_scalar
* raise ConversionError instead of ValidationError and use
get_param_name() for name
* instead of isinstance(value, str) or isinstance(value, unicode)
use isinstance(value, basestring)
* don't use pythonisms in public error messages (does not match any
of accepted formats: %s % self.accepted_formats)
* public error messages should be translatable
It should look something like this (untested, from top of my head):
type = datetime.datetime
type_error = _('must be a date/time 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: %s) %
(', '.join(self.accepted_formats))
raise ConversionError(name=self.get_param_name(),
index=index,
error=error)
return super(DateTime, self)._convert_scalar(value, index)
+if isinstance(value, DateTime):
+return str(value)
Return datetime object here please, or at least unicode in generalized
time format (or whatever the preferred format is, see above).
+elif isinstance(val, datetime.datetime):
+return val.strftime(%Y%m%dT%H:%M:%S)
Again, please use the preferred format here please.
Honza
--
Jan Cholasta
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0137] ipalib: Add DateTime parameter
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.
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 = [
['-MM-DD', /^(\d{4})-(\d{2})-(\d{2})$/],
['MMDD',/^(\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
['MMDDHHmmssZ',/^(\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.
Few nitpicks about DateTime implementation:
* you should call Param._convert_scalar from DateTime._convert_scalar
* raise ConversionError instead of ValidationError and use
get_param_name() for name
* instead of isinstance(value, str) or isinstance(value, unicode)
use isinstance(value, basestring)
* don't use pythonisms in public error messages (does not match any
of accepted formats: %s % self.accepted_formats)
* public error messages should be translatable
It should look something like this (untested, from top of my head):
type = datetime.datetime
type_error = _('must be a date/time 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: %s) %
(', '.join(self.accepted_formats))
raise ConversionError(name=self.get_param_name(),
index=index,
error=error)
return super(DateTime, self)._convert_scalar(value, index)
+if isinstance(value, DateTime):
+return str(value)
Return datetime object here please, or at least unicode in generalized
time format (or whatever the preferred format is, see above).
+elif isinstance(val, datetime.datetime):
+return val.strftime(%Y%m%dT%H:%M:%S)
Again, please use the preferred format here please.
Honza
--
Petr Vobornik
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0137] ipalib: Add DateTime parameter
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. Nathaniel ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0137] ipalib: Add DateTime parameter
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
From 26a57febd0a1b920cb0857f3a12912bb69c82d90 Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Thu, 9 Jan 2014 11:14:56 +0100
Subject: [PATCH 137/140] 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 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 without 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
---
ipalib/__init__.py | 2 +-
ipalib/parameters.py | 45 +
ipalib/rpc.py| 14 --
ipapython/ipaldap.py | 3 +++
4 files changed, 61 insertions(+), 3 deletions(-)
diff --git a/ipalib/__init__.py b/ipalib/__init__.py
index ab89ab77ec94603d242e56436021c9b6ed8663cb..5dfeab3326f0a7b77120f2ef560cf08e8b9aa704 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, Property
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 757c185655bbe1b586fbf088256891a7e7558876..eb5d57fc8da3a1f3796abf6c9d08412ed2181dae 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
@@ -1602,6 +1603,50 @@ class File(Str):
('noextrawhitespace', bool, False),
)
+class DateTime(Param):
+
+DateTime parameter type.
+
+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 without in the following format:
+ '%Y%m%d%H%M%SZ'
+
+Refer to the `man strftime` for the explanations for the %Y,%m,%d,%H.%M,%S.
+
+
+accepted_formats = ['%Y-%m-%dT%H:%M:%S',
+'%Y-%m-%dT%H:%M',
+'%Y%m%dT%H:%M:%S',
+'%Y%m%dT%H:%M',
+'%Y%m%d%H%M%SZ']
+
+type = datetime.datetime
+
+def _convert_scalar(self, value, index=None):
+if isinstance(value, datetime.datetime):
+return value
+elif isinstance(value, str) or isinstance(value, unicode):
+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: %s % self.accepted_formats
+
+raise ValidationError(name=self.name,
+ value=value,
+
