[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