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 = [
['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.
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
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel
