[Da-tools-discuss] Some ud-ldap patches

Stephen Gran sgran at debian.org
Sat May 9 10:34:56 UTC 2009


This one time, at band camp, Peter Palfrader said:
> On Sat, 09 May 2009, Stephen Gran wrote:
> > +
> > +   status = GetAttr(DnRecord,"accountStatus", None)
> > +   if status is None:
> > +      return False
> > +
> > +   if status.find("inactive") != -1:
> > +      return True
> > +
> > +   if status.find("memorial") != -1:
> > +      return True
> > +
> > +   if status.find("retiring") != -1:
> > +      line = status.split()
> > +      # We'll give them a few extra days over what we said
> 
> Substring matches probably are not the best thing to use here.  Can we
> just split the status into two strings, "status" and "date" like you
> did for the retiring case anyway and use that?

Sure.  That seems like a reasonable plan.

> > +      if IsRetired(x):
> > +         continue
> > +
> 
> > +      if IsRetired(x):
> > +         continue
> > +
> 
> > +      if IsRetired(x):
> > +         continue
> > +
> 
> >     for x in PasswdAttrs:
> > +      if IsRetired(x):
> > +         continue
> > +
> 
> ...
> ...
> ..
> 
> Somehow this seems wrong.  Just like the thing were we check if a
> password is locked a gazillian times.  Or the thing where we check if
> the uid is root.  Or the thing where we check if lengths are above a
> certain boundary.
> 
> This isn't your fault of course, but while we're it we should fix this.
> Mabye filtering them all in one place, and one place only.
> 
> Without looking at the code all too closely, or at all, I can see a few
> lists we would want to end up with:
> 
> - all users, with the exception of uid=0 and too long values for some
>   things.  This sanitizing step should be applied to the password list
>   immediately when we fetch it from ldap, and should be the basis for
>   all other lists.
> - everybody who should get listed in the email forward.  That's
>   basically everyone who is not IsRetired().
> - everybody whose account is not locked.

Yes, the code definitely needs a refactor - it's been growing functions
for a while without any thought to efficiency or design.  I'll see what
I can do about that.

Cheers,
-- 
 -----------------------------------------------------------------
|   ,''`.                                            Stephen Gran |
|  : :' :                                        sgran at debian.org |
|  `. `'                        Debian user, admin, and developer |
|    `-                                     http://www.debian.org |
 -----------------------------------------------------------------
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.alioth.debian.org/pipermail/da-tools-discuss/attachments/20090509/3e098d25/attachment.pgp>


More information about the Da-tools-discuss mailing list