Avoid connection attempt via ldap with unconfigured ldap_authentification

[ced1, 2009-09-29 18:31:23]

In fact it is not this file the issue, it is search of ModelSingleton that
returns int if limit == 1 which is not the behavior of the search of
ModelStorage.

[yangoon1, 2009-09-29 22:13:47]

On 2009/09/29 18:31:23, ced wrote:
> In fact it is not this file the issue, it is search of ModelSingleton that
> returns int if limit == 1 which is not the behavior of the search of
> ModelStorage.

Background of this patch:
Fallback to normal login has to be guaranteed in any case of failure of LDAP
login. So for me it is advisable to put the whole LDAP login block into the try:
statement, independent form other fixes to be made additionally.

[ced1, 2009-09-30 06:29:27]

On 2009/09/29 22:13:47, yangoon wrote:
> On 2009/09/29 18:31:23, ced wrote:
> > In fact it is not this file the issue, it is search of ModelSingleton that
> > returns int if limit == 1 which is not the behavior of the search of
> > ModelStorage.
> 
> Background of this patch:
> Fallback to normal login has to be guaranteed in any case of failure of LDAP
> login. So for me it is advisable to put the whole LDAP login block into the
try:
> statement, independent form other fixes to be made additionally.

No, it is bad to put too much stuff in try/except because it will be harder to
debug, we will miss some bug.
The try/except statement here is too catch exception due to tier party like the
LDAP connection.

[yangoon1, 2009-09-30 09:39:40]

On 2009/09/30 06:29:27, ced wrote:
> On 2009/09/29 22:13:47, yangoon wrote:
> > On 2009/09/29 18:31:23, ced wrote:
> > > In fact it is not this file the issue, it is search of ModelSingleton that
> > > returns int if limit == 1 which is not the behavior of the search of
> > > ModelStorage.
> > 
> > Background of this patch:
> > Fallback to normal login has to be guaranteed in any case of failure of LDAP
> > login. So for me it is advisable to put the whole LDAP login block into the
> try:
> > statement, independent form other fixes to be made additionally.
> 
> No, it is bad to put too much stuff in try/except because it will be harder to
> debug, we will miss some bug.
> The try/except statement here is too catch exception due to tier party like
the
> LDAP connection.

Ok, so do you want it to be fixed in res.py to check for the return value or do
you want to change return value of ModelSingleton to suit ModelStorage?

[ced1, 2009-09-30 09:41:18]

On 2009/09/30 09:39:40, yangoon wrote:
> On 2009/09/30 06:29:27, ced wrote:
> > On 2009/09/29 22:13:47, yangoon wrote:
> > > On 2009/09/29 18:31:23, ced wrote:
> > > > In fact it is not this file the issue, it is search of ModelSingleton
that
> > > > returns int if limit == 1 which is not the behavior of the search of
> > > > ModelStorage.
> > > 
> > > Background of this patch:
> > > Fallback to normal login has to be guaranteed in any case of failure of
LDAP
> > > login. So for me it is advisable to put the whole LDAP login block into
the
> > try:
> > > statement, independent form other fixes to be made additionally.
> > 
> > No, it is bad to put too much stuff in try/except because it will be harder
to
> > debug, we will miss some bug.
> > The try/except statement here is too catch exception due to tier party like
> the
> > LDAP connection.
> 
> Ok, so do you want it to be fixed in res.py to check for the return value or
do
> you want to change return value of ModelSingleton to suit ModelStorage?

It is clear that it is ModelSingleton that must be fixed.