Patch submission version 4

[Jelmer Vernooij]

Hi Stéphane,

On Wed, Oct 09, 2013 at 12:20:21PM +0200, Stéphane PURNELLE wrote:
> In attached file, patch in GIT format with some typo correction
> 
> could you review and push in your master (samba4.1) if ok.

I think the general idea behind this is a good. Some minor
comments, mostly style related:

+
+            """ if gid_number, we must verify if not already exist in SAM
+            """

Please use # for comments, rather than unassigned strings. The latter
are only used for docstrings.

+            if gid_number:
+                domain_dn = samdb.domain_dn()
+                res = samdb.search(domain_dn,
scope=ldb.SCOPE_SUBTREE,
+                    expression=("(&(gidNumber=%i))"
+                    % (gid_number)),attrs=["gid"])
+
+                if (len(res) != 0):
+                    raise Exception("gidNumber already exist\n")

Is it actually necessary to check this, or would we just get an
exception if the gidNumber was already used considering the 

+	 samdb.newgroup(groupname, groupou=groupou, grouptype = gtype,
Please don't add spaces in between for keyword arguments
(grouptype=gtype rather than "grouptype = gtype").

Related to this, I realize this might be a pattern you've copied from
elsewhere, but do we really need the ldbmessage2 construction? Is
there any reason we can't just do those assignments directly in the LdbMessage
that is being added?

Cheers,

Jelmer

> 
> -----------------------------------
> Stéphane PURNELLE                         Admin. Systèmes et Réseaux 
> Service Informatique       Corman S.A.           Tel : 00 32 (0)87/342467
> 
> Andrew Bartlett <abartlet at samba.org> wrote on 09/10/2013 04:44:25:
> 
> > De : Andrew Bartlett <abartlet at samba.org>
> > A : Stéphane PURNELLE <stephane.purnelle at corman.be>, 
> > Cc : samba-technical at samba.org
> > Date : 09/10/2013 04:51
> > Objet : Re: Patch submission version 4
> > 
> > On Wed, 2013-10-02 at 13:32 +0200, Stéphane PURNELLE wrote:
> > > Independent of usage of posixAccount objectclass and RFC2307, 
> uidNumber, 
> > > gidNumber, ...
> > > 
> > > This patchset permit to set uidNumber and gidNumber using samba-tool 
> and 
> > > verify if uidNumber or gidNumber is already exist or not.
> > 
> > Thank you for looking into this.
> > 
> > One issue is that we cannot know if a uid or gid is in use, if the
> > directory is not in a good replication state right now.   So, while a
> > very valuable too, it isn't able to be a perfect failsafe. 
> > 
> > On the patch itself, you have a number of style and indenting issues.
> > In particular, make sure you always have a space after a , and that you
> > lay the functions out in the same way as other examples in the code.
> > 
> > It would also really help if you could also commit the changes into a
> > git repository (with git commit -s), and run git-format-patch -1 to
> > produce the patch.  That way, we have your signed off patch with commit
> > message all sorted, ready to get into master. 
> > 
> > Thanks,
> > 
> > Andrew Bartlett
> > -- 
> > Andrew Bartlett
> > http://samba.org/~abartlet/
> > Authentication Developer, Samba Team           http://samba.org
> > Samba Developer, Catalyst IT                   http://catalyst.net.nz
> > 
> >