[SCM] Samba Shared Repository - branch master updated

[Jelmer Vernooij]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


Hi Matthieu,

On 02/11/11 15:15, Matthieu Patou wrote:
> 
> +#tests on sites +class SimpleSitesTests(SitesBaseTests): + + + def
> test_create(self): + """test creation of 1 site""" + +
> self.ldb_admin.transaction_start() + ok =
> sites.create_site(self.ldb_admin,
self.ldb_admin.get_config_basedn(),
> + "testsamba") + self.ldb_admin.transaction_commit() +
> self.assertTrue(ok) + ok = False + try: + ok =
> sites.create_site(self.ldb_admin,
self.ldb_admin.get_config_basedn(),
> + "testsamba") + self.assertFalse(ok) + except: +
> self.assertFalse(ok)
This looks dodgy. If sites.create_site raises *any* exception (even a
SyntaxError!), then this test still passes since ok is False by default.

It's almost never right to use "except:" - and if you do use it, please
use "raise" to re-raise the exception. "except:" catches every kind of
exception, including KeyboardInterrupt and SyntaxError.

> diff --git a/source4/scripting/python/samba/netcmd/sites.py
b/source4/scripting/python/samba/netcmd/sites.py
> new file mode 100644 index 0000000..a63b524 --- /dev/null +++
> b/source4/scripting/python/samba/netcmd/sites.py @@ -0,0 +1,96 @@ 
> + +#!/usr/bin/env python +# +# sites management +# +# Copyright
> Matthieu Patou <mat at matws.net> 2011 +# +# This program is free
> software; you can redistribute it and/or modify +# it under the
> terms of the GNU General Public License as published by +# the Free
> Software Foundation; either version 3 of the License, or +# (at
> your option) any later version. +# +# This program is distributed
> in the hope that it will be useful, +# but WITHOUT ANY WARRANTY;
> without even the implied warranty of +# MERCHANTABILITY or FITNESS
> FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for
> more details. +# +# You should have received a copy of the GNU
> General Public License +# along with this program. If not, see
> <http://www.gnu.org/licenses/>. +# + + + +import os +from samba
> import sites +from samba import Ldb +from samba.auth import
> system_session +from samba.netcmd import ( + Command, +
> CommandError, + SuperCommand + ) + + +class
> cmd_sites_create(Command): + """Create a new site""" + + synopsis =
> "%prog <site> [options]" + + takes_args = ["sitename"] + + def
> run(self, sitename, sambaopts=None, credopts=None,
> versionopts=None): + lp = sambaopts.get_loadparm() + creds =
> credopts.get_credentials(lp, fallback_machine=True) + name =
> "sam.ldb" + path = lp.get("private dir") + url = os.path.join(path,
> name)
It should be shorter to use lp.private_path("sam.ldb").

> 
> + if not os.path.exists(url): + raise CommandError("secret database
> not found at %s " % url)
This says secrets database, but it looks like it's actually about the
sam ?

> 
> + samdb = Ldb(url=url, session_info=system_session(), +
> credentials=creds, lp=lp)
You probably want to use SamDB rather than Ldb here.
> 
> + + samdb.transaction_start() + ok = sites.create_site(samdb,
> samdb.get_config_basedn(), sitename) + samdb.transaction_commit()
This needs a try/except/finally otherwise we'll get a nasty warning
about transactions that weren't closed.
> 
> +class cmd_sites_delete(Command): + """Delete a new site""" +
The same comments as mentioned above apply to cmd_sites_delete.

> 
> diff --git a/source4/scripting/python/samba/sites.py
b/source4/scripting/python/samba/sites.py
> new file mode 100644 index 0000000..d1d0e75 --- /dev/null +++
> b/source4/scripting/python/samba/sites.py @@ -0,0 +1,63 @@ 
> +#!/usr/bin/env python +# +# python site manipulation code +#
> Copyright Matthieu Patou <mat at matws.net> 2011 +# +# This program is
> free software; you can redistribute it and/or modify +# it under
> the terms of the GNU General Public License as published by +# the
> Free Software Foundation; either version 3 of the License, or +#
> (at your option) any later version. +# +# This program is
> distributed in the hope that it will be useful, +# but WITHOUT ANY
> WARRANTY; without even the implied warranty of +# MERCHANTABILITY
> or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public
> License for more details. +# +# You should have received a copy of
> the GNU General Public License +# along with this program. If not,
> see <http://www.gnu.org/licenses/>. +# + +"""Manipulating
> sites.""" + +import ldb +from ldb import FLAG_MOD_ADD + +def
> create_site(samdb, configDn, siteName):
It would be nice to have a docstring on this method. It seems to always
return True, but what does that actually indicate? It might be nicer to
not return anything specific but just raise exceptions in case problems
occur.

Cheers,

Jelmer
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJOsVK6AAoJEACAbyvXKaRXl00P/iWysJZtCdfVBBBJpwLbMiVQ
+uA+gM250TBnSF6nvek4aYtdg8P+IB8HxlJ0pvige6cJl0hziPWQNsSSNAItzzyn
uZA50sxR0RY+I969oegGS6mN107gB5T8EG5+b6PoXJUjE1ez5abHnUULJxHjrK+R
3ozZeXDDvAdGwNeRpOhfQF0kPhh5k1mTZMZmQ1KktG7ovPANGmZAduOARBJlbcas
mHPetRu4+xFJMBHntF+18UTqGRniyk05ROlVwfSdV0ve3V5gu+ZZSbYSivRpQYqK
AwucGZ3O9jKDQ/yufyLFuPNkgAqmF8dpVz44ikLJyPqc03SaxXwXmgItQGZuDZ+G
1ZiXIymbutYDN3ziU5NhTlWCrgjyUGNUBFTbQdgPGTUnx9zsUQeqU0ASBWotWYV8
PJwA5+Vyj+JIq7f1zS3xQsgao9sPsHz626u+N5jqESZzZ7Qdj3zgKGUcPUj9HR/2
AYleBqy35MX4ZU2C+jkeOmS3ol+CGTF09yc95cgTFREpqORuKEuveWWgLJFZjz5y
7JUJJUSiEF1gOZi//3wilx99aXml3ZVAEnoTA/zRPMZFOOsDSXOhMbYuK0ys/X5L
yPBzAhFk2z+slj0hGRfvJSrX8siE/DrDnPIHYzTYlIaq5opFBu94K9z8m5MW84oh
7ui+ySGLS7gW735lE9eK
=LsdB
-----END PGP SIGNATURE-----