Re: [Freeipa-devel] [PATCH 0236] ipaldap: Fallback to string if datetime conversion went wrong
On 07/01/2014 02:41 PM, Rob Crittenden wrote: > Tomas Babej wrote: >> On 07/01/2014 12:19 PM, Martin Kosek wrote: >>> On 06/26/2014 10:44 AM, Jan Cholasta wrote: On 26.6.2014 10:39, Petr Viktorin wrote: > On 06/26/2014 10:33 AM, Jan Cholasta wrote: >> On 26.6.2014 09:40, Petr Viktorin wrote: >>> On 06/26/2014 09:33 AM, Jan Cholasta wrote: On 26.6.2014 09:21, Petr Viktorin wrote: > On 06/26/2014 08:30 AM, Jan Cholasta wrote: >> On 25.6.2014 18:25, Petr Viktorin wrote: >>> On 06/25/2014 05:29 PM, Jan Cholasta wrote: Hi, On 25.6.2014 17:17, Tomas Babej wrote: > Hi, > > Our datetime conversion does not support full LDAP Generalized > time syntax. In the unsupported cases, we should fall back > to string representation of the attribute. > > In particular, '0' is used to denote no value of LDAP generalized > time attribute. > > https://fedorahosted.org/freeipa/ticket/4350 NACK, this beats the purpose of decoding of the values, because it requires you to check the type of the value before using it. Instead, you should either fix the code that uses the nsds5ReplicaLastUpdate{Start,End} attributes to access their raw value directly, or exclude the attributes from decoding to datetime by overriding their type in IPASimpleLDAPObject._SYNTAX_OVERRIDE. Honza > [...] >>> The problem is that "unset" is a valid value here, >> It is not, according to Generalized Time syntax (RFC 4517 section >> 3.3.13) "0" is an invalid value and we should treat it the same way as >> any other invalid value (hence my original suggestion). > OK, in that case ignore what I said here. > > So am I correct that 389-ds storing a value that doesn't comply with the > attribute's syntax? Should we file a DS bug? > AFAIK syntax checks are done only on LDAP adds and mods, so unless we are setting "0" somewhere and DS does not complain, it is not a bug. >>> What is the final resolution here? Do we plan to ignore the attribute >>> conversion for these attributes as, fixing DS or are we fixing the >>> framework? >>> >>> Surprisingly, I did not see the filed failure any more in my >>> ipa-replica-install tests nor in the latest CI run, I wonder if anything >>> changed in our code or if the issue is intermittent. Anyway, I am willing to >>> close this ticket if this is no longer a problem. >>> >>> Martin >>> >>> ___ >>> Freeipa-devel mailing list >>> Freeipa-devel@redhat.com >>> https://www.redhat.com/mailman/listinfo/freeipa-devel >> Issue is not 100% reproducible in my testing. The underlying problem >> here is that sometimes the values of nsds5replicalastupdatestart and >> nsds5replicalastupdateend attributes are not being set during >> replication (intermittent issue) and thus 389 falls back and returns '0' >> if any of these attributes were explicitily requested, which is not a >> valid value for LDAP Generalized time. >> >> The reason you're not hitting the conversion errors is probably that >> during replication nsds5replicalastupdatestart and >> nsds5replicalastupdateend were assigned proper values. You can check >> that in the replication log (search for "Replication Update in progress: >> %s: status: %s: start: %d: end: %d" in the logs). >> >> I am checking with Ludvig, but if this behaviour is specific to the >> nsds5replicalastupdatestart and nsds5replicalastupdateend I'd go with >> the SYNTAX_OVERRIDE solution. >> >> However, it is worth noting that these conversion errors are harmless >> themselves. >> > IIRC I also saw this with: > > ipa-replica-manage list -v `hostname` > > That may be easier to reproduce/validate. > > rob Please see my patch 238 on the list, which obsoletes this thread. -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0236] ipaldap: Fallback to string if datetime conversion went wrong
Tomas Babej wrote: > > On 07/01/2014 12:19 PM, Martin Kosek wrote: >> On 06/26/2014 10:44 AM, Jan Cholasta wrote: >>> On 26.6.2014 10:39, Petr Viktorin wrote: On 06/26/2014 10:33 AM, Jan Cholasta wrote: > On 26.6.2014 09:40, Petr Viktorin wrote: >> On 06/26/2014 09:33 AM, Jan Cholasta wrote: >>> On 26.6.2014 09:21, Petr Viktorin wrote: On 06/26/2014 08:30 AM, Jan Cholasta wrote: > On 25.6.2014 18:25, Petr Viktorin wrote: >> On 06/25/2014 05:29 PM, Jan Cholasta wrote: >>> Hi, >>> >>> On 25.6.2014 17:17, Tomas Babej wrote: Hi, Our datetime conversion does not support full LDAP Generalized time syntax. In the unsupported cases, we should fall back to string representation of the attribute. In particular, '0' is used to denote no value of LDAP generalized time attribute. https://fedorahosted.org/freeipa/ticket/4350 >>> NACK, this beats the purpose of decoding of the values, because it >>> requires you to check the type of the value before using it. >>> >>> Instead, you should either fix the code that uses the >>> nsds5ReplicaLastUpdate{Start,End} attributes to access their raw >>> value >>> directly, or exclude the attributes from decoding to datetime by >>> overriding their type in IPASimpleLDAPObject._SYNTAX_OVERRIDE. >>> >>> Honza >>> [...] >> The problem is that "unset" is a valid value here, > It is not, according to Generalized Time syntax (RFC 4517 section > 3.3.13) "0" is an invalid value and we should treat it the same way as > any other invalid value (hence my original suggestion). OK, in that case ignore what I said here. So am I correct that 389-ds storing a value that doesn't comply with the attribute's syntax? Should we file a DS bug? >>> AFAIK syntax checks are done only on LDAP adds and mods, so unless we are >>> setting "0" somewhere and DS does not complain, it is not a bug. >> What is the final resolution here? Do we plan to ignore the attribute >> conversion for these attributes as, fixing DS or are we fixing the framework? >> >> Surprisingly, I did not see the filed failure any more in my >> ipa-replica-install tests nor in the latest CI run, I wonder if anything >> changed in our code or if the issue is intermittent. Anyway, I am willing to >> close this ticket if this is no longer a problem. >> >> Martin >> >> ___ >> Freeipa-devel mailing list >> Freeipa-devel@redhat.com >> https://www.redhat.com/mailman/listinfo/freeipa-devel > > Issue is not 100% reproducible in my testing. The underlying problem > here is that sometimes the values of nsds5replicalastupdatestart and > nsds5replicalastupdateend attributes are not being set during > replication (intermittent issue) and thus 389 falls back and returns '0' > if any of these attributes were explicitily requested, which is not a > valid value for LDAP Generalized time. > > The reason you're not hitting the conversion errors is probably that > during replication nsds5replicalastupdatestart and > nsds5replicalastupdateend were assigned proper values. You can check > that in the replication log (search for "Replication Update in progress: > %s: status: %s: start: %d: end: %d" in the logs). > > I am checking with Ludvig, but if this behaviour is specific to the > nsds5replicalastupdatestart and nsds5replicalastupdateend I'd go with > the SYNTAX_OVERRIDE solution. > > However, it is worth noting that these conversion errors are harmless > themselves. > IIRC I also saw this with: ipa-replica-manage list -v `hostname` That may be easier to reproduce/validate. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0236] ipaldap: Fallback to string if datetime conversion went wrong
On 07/01/2014 12:19 PM, Martin Kosek wrote: > On 06/26/2014 10:44 AM, Jan Cholasta wrote: >> On 26.6.2014 10:39, Petr Viktorin wrote: >>> On 06/26/2014 10:33 AM, Jan Cholasta wrote: On 26.6.2014 09:40, Petr Viktorin wrote: > On 06/26/2014 09:33 AM, Jan Cholasta wrote: >> On 26.6.2014 09:21, Petr Viktorin wrote: >>> On 06/26/2014 08:30 AM, Jan Cholasta wrote: On 25.6.2014 18:25, Petr Viktorin wrote: > On 06/25/2014 05:29 PM, Jan Cholasta wrote: >> Hi, >> >> On 25.6.2014 17:17, Tomas Babej wrote: >>> Hi, >>> >>> Our datetime conversion does not support full LDAP Generalized >>> time syntax. In the unsupported cases, we should fall back >>> to string representation of the attribute. >>> >>> In particular, '0' is used to denote no value of LDAP generalized >>> time attribute. >>> >>> https://fedorahosted.org/freeipa/ticket/4350 >> NACK, this beats the purpose of decoding of the values, because it >> requires you to check the type of the value before using it. >> >> Instead, you should either fix the code that uses the >> nsds5ReplicaLastUpdate{Start,End} attributes to access their raw >> value >> directly, or exclude the attributes from decoding to datetime by >> overriding their type in IPASimpleLDAPObject._SYNTAX_OVERRIDE. >> >> Honza >> >>> [...] > The problem is that "unset" is a valid value here, It is not, according to Generalized Time syntax (RFC 4517 section 3.3.13) "0" is an invalid value and we should treat it the same way as any other invalid value (hence my original suggestion). >>> OK, in that case ignore what I said here. >>> >>> So am I correct that 389-ds storing a value that doesn't comply with the >>> attribute's syntax? Should we file a DS bug? >>> >> AFAIK syntax checks are done only on LDAP adds and mods, so unless we are >> setting "0" somewhere and DS does not complain, it is not a bug. > What is the final resolution here? Do we plan to ignore the attribute > conversion for these attributes as, fixing DS or are we fixing the framework? > > Surprisingly, I did not see the filed failure any more in my > ipa-replica-install tests nor in the latest CI run, I wonder if anything > changed in our code or if the issue is intermittent. Anyway, I am willing to > close this ticket if this is no longer a problem. > > Martin > > ___ > Freeipa-devel mailing list > Freeipa-devel@redhat.com > https://www.redhat.com/mailman/listinfo/freeipa-devel Issue is not 100% reproducible in my testing. The underlying problem here is that sometimes the values of nsds5replicalastupdatestart and nsds5replicalastupdateend attributes are not being set during replication (intermittent issue) and thus 389 falls back and returns '0' if any of these attributes were explicitily requested, which is not a valid value for LDAP Generalized time. The reason you're not hitting the conversion errors is probably that during replication nsds5replicalastupdatestart and nsds5replicalastupdateend were assigned proper values. You can check that in the replication log (search for "Replication Update in progress: %s: status: %s: start: %d: end: %d" in the logs). I am checking with Ludvig, but if this behaviour is specific to the nsds5replicalastupdatestart and nsds5replicalastupdateend I'd go with the SYNTAX_OVERRIDE solution. However, it is worth noting that these conversion errors are harmless themselves. -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0236] ipaldap: Fallback to string if datetime conversion went wrong
On 06/26/2014 10:44 AM, Jan Cholasta wrote: > On 26.6.2014 10:39, Petr Viktorin wrote: >> On 06/26/2014 10:33 AM, Jan Cholasta wrote: >>> On 26.6.2014 09:40, Petr Viktorin wrote: On 06/26/2014 09:33 AM, Jan Cholasta wrote: > On 26.6.2014 09:21, Petr Viktorin wrote: >> On 06/26/2014 08:30 AM, Jan Cholasta wrote: >>> On 25.6.2014 18:25, Petr Viktorin wrote: On 06/25/2014 05:29 PM, Jan Cholasta wrote: > Hi, > > On 25.6.2014 17:17, Tomas Babej wrote: >> Hi, >> >> Our datetime conversion does not support full LDAP Generalized >> time syntax. In the unsupported cases, we should fall back >> to string representation of the attribute. >> >> In particular, '0' is used to denote no value of LDAP generalized >> time attribute. >> >> https://fedorahosted.org/freeipa/ticket/4350 > > NACK, this beats the purpose of decoding of the values, because it > requires you to check the type of the value before using it. > > Instead, you should either fix the code that uses the > nsds5ReplicaLastUpdate{Start,End} attributes to access their raw > value > directly, or exclude the attributes from decoding to datetime by > overriding their type in IPASimpleLDAPObject._SYNTAX_OVERRIDE. > > Honza > >> [...] The problem is that "unset" is a valid value here, >>> >>> It is not, according to Generalized Time syntax (RFC 4517 section >>> 3.3.13) "0" is an invalid value and we should treat it the same way as >>> any other invalid value (hence my original suggestion). >> >> OK, in that case ignore what I said here. >> >> So am I correct that 389-ds storing a value that doesn't comply with the >> attribute's syntax? Should we file a DS bug? >> > > AFAIK syntax checks are done only on LDAP adds and mods, so unless we are > setting "0" somewhere and DS does not complain, it is not a bug. What is the final resolution here? Do we plan to ignore the attribute conversion for these attributes as, fixing DS or are we fixing the framework? Surprisingly, I did not see the filed failure any more in my ipa-replica-install tests nor in the latest CI run, I wonder if anything changed in our code or if the issue is intermittent. Anyway, I am willing to close this ticket if this is no longer a problem. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0236] ipaldap: Fallback to string if datetime conversion went wrong
On 26.6.2014 10:39, Petr Viktorin wrote: On 06/26/2014 10:33 AM, Jan Cholasta wrote: On 26.6.2014 09:40, Petr Viktorin wrote: On 06/26/2014 09:33 AM, Jan Cholasta wrote: On 26.6.2014 09:21, Petr Viktorin wrote: On 06/26/2014 08:30 AM, Jan Cholasta wrote: On 25.6.2014 18:25, Petr Viktorin wrote: On 06/25/2014 05:29 PM, Jan Cholasta wrote: Hi, On 25.6.2014 17:17, Tomas Babej wrote: Hi, Our datetime conversion does not support full LDAP Generalized time syntax. In the unsupported cases, we should fall back to string representation of the attribute. In particular, '0' is used to denote no value of LDAP generalized time attribute. https://fedorahosted.org/freeipa/ticket/4350 NACK, this beats the purpose of decoding of the values, because it requires you to check the type of the value before using it. Instead, you should either fix the code that uses the nsds5ReplicaLastUpdate{Start,End} attributes to access their raw value directly, or exclude the attributes from decoding to datetime by overriding their type in IPASimpleLDAPObject._SYNTAX_OVERRIDE. Honza [...] The problem is that "unset" is a valid value here, It is not, according to Generalized Time syntax (RFC 4517 section 3.3.13) "0" is an invalid value and we should treat it the same way as any other invalid value (hence my original suggestion). OK, in that case ignore what I said here. So am I correct that 389-ds storing a value that doesn't comply with the attribute's syntax? Should we file a DS bug? AFAIK syntax checks are done only on LDAP adds and mods, so unless we are setting "0" somewhere and DS does not complain, it is not a bug. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0236] ipaldap: Fallback to string if datetime conversion went wrong
On 06/26/2014 10:33 AM, Jan Cholasta wrote: On 26.6.2014 09:40, Petr Viktorin wrote: On 06/26/2014 09:33 AM, Jan Cholasta wrote: On 26.6.2014 09:21, Petr Viktorin wrote: On 06/26/2014 08:30 AM, Jan Cholasta wrote: On 25.6.2014 18:25, Petr Viktorin wrote: On 06/25/2014 05:29 PM, Jan Cholasta wrote: Hi, On 25.6.2014 17:17, Tomas Babej wrote: Hi, Our datetime conversion does not support full LDAP Generalized time syntax. In the unsupported cases, we should fall back to string representation of the attribute. In particular, '0' is used to denote no value of LDAP generalized time attribute. https://fedorahosted.org/freeipa/ticket/4350 NACK, this beats the purpose of decoding of the values, because it requires you to check the type of the value before using it. Instead, you should either fix the code that uses the nsds5ReplicaLastUpdate{Start,End} attributes to access their raw value directly, or exclude the attributes from decoding to datetime by overriding their type in IPASimpleLDAPObject._SYNTAX_OVERRIDE. Honza [...] The problem is that "unset" is a valid value here, It is not, according to Generalized Time syntax (RFC 4517 section 3.3.13) "0" is an invalid value and we should treat it the same way as any other invalid value (hence my original suggestion). OK, in that case ignore what I said here. So am I correct that 389-ds storing a value that doesn't comply with the attribute's syntax? Should we file a DS bug? -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0236] ipaldap: Fallback to string if datetime conversion went wrong
On 26.6.2014 09:40, Petr Viktorin wrote: On 06/26/2014 09:33 AM, Jan Cholasta wrote: On 26.6.2014 09:21, Petr Viktorin wrote: On 06/26/2014 08:30 AM, Jan Cholasta wrote: On 25.6.2014 18:25, Petr Viktorin wrote: On 06/25/2014 05:29 PM, Jan Cholasta wrote: Hi, On 25.6.2014 17:17, Tomas Babej wrote: Hi, Our datetime conversion does not support full LDAP Generalized time syntax. In the unsupported cases, we should fall back to string representation of the attribute. In particular, '0' is used to denote no value of LDAP generalized time attribute. https://fedorahosted.org/freeipa/ticket/4350 NACK, this beats the purpose of decoding of the values, because it requires you to check the type of the value before using it. Instead, you should either fix the code that uses the nsds5ReplicaLastUpdate{Start,End} attributes to access their raw value directly, or exclude the attributes from decoding to datetime by overriding their type in IPASimpleLDAPObject._SYNTAX_OVERRIDE. Honza I agree that just returning a string when conversion fails is the wrong thing to do. I think that if LDAP generalized can contain the empty/invalid value, we should convert the '0' to what Python uses for that -- None. But None already means "empty attribute". I don't think you can get [None] currently, can you? None is the default default for get(), but I don't think that's a problem. You can't, but you can get it from single_value when the attribute is empty or assign it to an attribute to make it empty. I would very much prefer if we avoided None, because it will only create mess and possibly some hard to catch errors. The problem is that "unset" is a valid value here, It is not, according to Generalized Time syntax (RFC 4517 section 3.3.13) "0" is an invalid value and we should treat it the same way as any other invalid value (hence my original suggestion). and None is the Python representation for that. If it has to make single_value ambiguous, I'd be fine with that -- single_value is just a helper. But, shouldn't single_value[x] raise KeyError if the attribute is missing? It does. When attribute is empty/None, it is unset, not missing. There is old code which depends on this. As for setting, there is a difference between entry.single_value[x] = None and del entry[x] and we should use the correct one for each case. Yes, but until the old code is fixed, None has a special meaning and can't be used for anything else. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0236] ipaldap: Fallback to string if datetime conversion went wrong
On 06/26/2014 09:33 AM, Jan Cholasta wrote: On 26.6.2014 09:21, Petr Viktorin wrote: On 06/26/2014 08:30 AM, Jan Cholasta wrote: On 25.6.2014 18:25, Petr Viktorin wrote: On 06/25/2014 05:29 PM, Jan Cholasta wrote: Hi, On 25.6.2014 17:17, Tomas Babej wrote: Hi, Our datetime conversion does not support full LDAP Generalized time syntax. In the unsupported cases, we should fall back to string representation of the attribute. In particular, '0' is used to denote no value of LDAP generalized time attribute. https://fedorahosted.org/freeipa/ticket/4350 NACK, this beats the purpose of decoding of the values, because it requires you to check the type of the value before using it. Instead, you should either fix the code that uses the nsds5ReplicaLastUpdate{Start,End} attributes to access their raw value directly, or exclude the attributes from decoding to datetime by overriding their type in IPASimpleLDAPObject._SYNTAX_OVERRIDE. Honza I agree that just returning a string when conversion fails is the wrong thing to do. I think that if LDAP generalized can contain the empty/invalid value, we should convert the '0' to what Python uses for that -- None. But None already means "empty attribute". I don't think you can get [None] currently, can you? None is the default default for get(), but I don't think that's a problem. You can't, but you can get it from single_value when the attribute is empty or assign it to an attribute to make it empty. I would very much prefer if we avoided None, because it will only create mess and possibly some hard to catch errors. The problem is that "unset" is a valid value here, and None is the Python representation for that. If it has to make single_value ambiguous, I'd be fine with that -- single_value is just a helper. But, shouldn't single_value[x] raise KeyError if the attribute is missing? As for setting, there is a difference between entry.single_value[x] = None and del entry[x] and we should use the correct one for each case. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0236] ipaldap: Fallback to string if datetime conversion went wrong
On 26.6.2014 09:21, Petr Viktorin wrote: On 06/26/2014 08:30 AM, Jan Cholasta wrote: On 25.6.2014 18:25, Petr Viktorin wrote: On 06/25/2014 05:29 PM, Jan Cholasta wrote: Hi, On 25.6.2014 17:17, Tomas Babej wrote: Hi, Our datetime conversion does not support full LDAP Generalized time syntax. In the unsupported cases, we should fall back to string representation of the attribute. In particular, '0' is used to denote no value of LDAP generalized time attribute. https://fedorahosted.org/freeipa/ticket/4350 NACK, this beats the purpose of decoding of the values, because it requires you to check the type of the value before using it. Instead, you should either fix the code that uses the nsds5ReplicaLastUpdate{Start,End} attributes to access their raw value directly, or exclude the attributes from decoding to datetime by overriding their type in IPASimpleLDAPObject._SYNTAX_OVERRIDE. Honza I agree that just returning a string when conversion fails is the wrong thing to do. I think that if LDAP generalized can contain the empty/invalid value, we should convert the '0' to what Python uses for that -- None. But None already means "empty attribute". I don't think you can get [None] currently, can you? None is the default default for get(), but I don't think that's a problem. You can't, but you can get it from single_value when the attribute is empty or assign it to an attribute to make it empty. I would very much prefer if we avoided None, because it will only create mess and possibly some hard to catch errors. -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0236] ipaldap: Fallback to string if datetime conversion went wrong
On 06/26/2014 08:30 AM, Jan Cholasta wrote: On 25.6.2014 18:25, Petr Viktorin wrote: On 06/25/2014 05:29 PM, Jan Cholasta wrote: Hi, On 25.6.2014 17:17, Tomas Babej wrote: Hi, Our datetime conversion does not support full LDAP Generalized time syntax. In the unsupported cases, we should fall back to string representation of the attribute. In particular, '0' is used to denote no value of LDAP generalized time attribute. https://fedorahosted.org/freeipa/ticket/4350 NACK, this beats the purpose of decoding of the values, because it requires you to check the type of the value before using it. Instead, you should either fix the code that uses the nsds5ReplicaLastUpdate{Start,End} attributes to access their raw value directly, or exclude the attributes from decoding to datetime by overriding their type in IPASimpleLDAPObject._SYNTAX_OVERRIDE. Honza I agree that just returning a string when conversion fails is the wrong thing to do. I think that if LDAP generalized can contain the empty/invalid value, we should convert the '0' to what Python uses for that -- None. But None already means "empty attribute". I don't think you can get [None] currently, can you? None is the default default for get(), but I don't think that's a problem. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0236] ipaldap: Fallback to string if datetime conversion went wrong
On 25.6.2014 18:25, Petr Viktorin wrote: On 06/25/2014 05:29 PM, Jan Cholasta wrote: Hi, On 25.6.2014 17:17, Tomas Babej wrote: Hi, Our datetime conversion does not support full LDAP Generalized time syntax. In the unsupported cases, we should fall back to string representation of the attribute. In particular, '0' is used to denote no value of LDAP generalized time attribute. https://fedorahosted.org/freeipa/ticket/4350 NACK, this beats the purpose of decoding of the values, because it requires you to check the type of the value before using it. Instead, you should either fix the code that uses the nsds5ReplicaLastUpdate{Start,End} attributes to access their raw value directly, or exclude the attributes from decoding to datetime by overriding their type in IPASimpleLDAPObject._SYNTAX_OVERRIDE. Honza I agree that just returning a string when conversion fails is the wrong thing to do. I think that if LDAP generalized can contain the empty/invalid value, we should convert the '0' to what Python uses for that -- None. But None already means "empty attribute". -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0236] ipaldap: Fallback to string if datetime conversion went wrong
On 06/25/2014 05:29 PM, Jan Cholasta wrote: Hi, On 25.6.2014 17:17, Tomas Babej wrote: Hi, Our datetime conversion does not support full LDAP Generalized time syntax. In the unsupported cases, we should fall back to string representation of the attribute. In particular, '0' is used to denote no value of LDAP generalized time attribute. https://fedorahosted.org/freeipa/ticket/4350 NACK, this beats the purpose of decoding of the values, because it requires you to check the type of the value before using it. Instead, you should either fix the code that uses the nsds5ReplicaLastUpdate{Start,End} attributes to access their raw value directly, or exclude the attributes from decoding to datetime by overriding their type in IPASimpleLDAPObject._SYNTAX_OVERRIDE. Honza I agree that just returning a string when conversion fails is the wrong thing to do. I think that if LDAP generalized can contain the empty/invalid value, we should convert the '0' to what Python uses for that -- None. -- Petr³ ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0236] ipaldap: Fallback to string if datetime conversion went wrong
Hi, On 25.6.2014 17:17, Tomas Babej wrote: Hi, Our datetime conversion does not support full LDAP Generalized time syntax. In the unsupported cases, we should fall back to string representation of the attribute. In particular, '0' is used to denote no value of LDAP generalized time attribute. https://fedorahosted.org/freeipa/ticket/4350 NACK, this beats the purpose of decoding of the values, because it requires you to check the type of the value before using it. Instead, you should either fix the code that uses the nsds5ReplicaLastUpdate{Start,End} attributes to access their raw value directly, or exclude the attributes from decoding to datetime by overriding their type in IPASimpleLDAPObject._SYNTAX_OVERRIDE. Honza -- Jan Cholasta ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel