-----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 <m...@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 <m...@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-----