[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/

iQIcBAEBAgAGBQJOsVKTAAoJEACAbyvXKaRXewoP/inmGReeG3tiz8pt9kXcrLNh
dyFQEJf/8ckiGpgqZRT8wbqUgnq83VdXdg+6k0x8aPH4aNR+kvZ3v9iBSMtM9Uwa
dl5Z7yl7mglNqU8xawB7QiPWvhElJYrkrKSR8fqgTGbAsbsRNYU7joBH+NiWo2iK
MZNcq7oVr5+H02wHyzh+jY7x0Iadfids3MxO7mh5cvIX4auUlTjklQ+kEKwHEENN
E9p9Q305vYx2N3hOkU/xq6e2IvV98JnRTdYda/eYOMCw5/RUnBWlSINWsHrYk4AU
FqwZ3vx244P2hJvrMnJtrJ1/UQNF4G7BBG5tOrshbFcmRKhkIdonR8o5JAs7ovWz
k5d7sCCZY6ohQ594bz1Uqn5NLq9B7bO6YAPlabQIQlQrNpOl3vwvdFrA4PXgT1Fh
oFG5cJQAeQbVoOZhD6tGVzUcijn8lThETjWDKv7JNPOiA7uzYalKJ52pfk9B6HyV
hiXOQOOZmg7eDMWAxGHGQ1bCj6DrQ1pZNwKJEFSE3+Gg5DagkU3BhGYGgvsien3A
c58MyUYMimxLJEY5U+ms4WuqdYsZ3zGWVCV7+R0tsGpHl/yEn4C89ci7dymUq4nV
OfZCscwn/eti2R2jekEuM0Qyc16+o5kCzCQ5TA2UvjHlU/w4l3i/b5CHL3FK2+Oz
rK4+WwiDB8+I6BJ9F+4K
=pZ1x
-----END PGP SIGNATURE-----