Patch for LDB SAMBA python bindings

Jelmer Vernooij jelmer at samba.org
Wed Nov 4 17:09:57 MST 2009


Hi Matthias,

Thanks for the updated patch. Can you please also reply to my other
question: What is the public API for setting the wrap_url useful for
exactly? Who would call it?

If it really should be public it would be nice to have a unit test for
it, if it should rather be private it probably needs a caller (and bonus
points for a test in that case).

Cheers,

Jelmer

On Wed, 2009-11-04 at 22:09 +0100, Matthias Dieter Wallnöfer wrote:
> Latest version of the patch.
> 
> Matthias
> 
> Jelmer Vernooij wrote:
> > Hi Matthias,
> >
> > E.g. an example would be:
> >
> > def foo(a, b=10):
> > 	print "a: %d, b: %d" % (a, b)
> >
> > you can then call foo as:
> >
> > foo(42): a: 42, b: 10
> > foo(30, 70): a: 30, b: 70
> > foo(a=60): a: 60, b: 10
> > foo(b=80,a=20): a: 20, b: 80
> >
> > Cheers,
> >
> > Jelmer
> >
> > On Fri, Oct 30, 2009 at 11:49:35AM +0000, Matthias Dieter Wallnöfer wrote:
> >    
> >> Jelmer, since I'm not a python expert, please give me a concrete example and point out the code locations so I can write a patch regarding the "optional argument". Then I can try to improve my patch.
> >>
> >> Matthias
> >>
> >> --- Jelmer Vernooij<jelmer at samba.org>  schrieb am Fr, 30.10.2009:
> >>
> >> Von: Jelmer Vernooij<jelmer at samba.org>
> >> Betreff: Re: Patch for LDB SAMBA python bindings
> >> An: "Matthias Dieter Wallnöfer"<mdw at samba.org>
> >> CC: "samba-technical"<samba-technical at lists.samba.org>
> >> Datum: Freitag, 30. Oktober 2009, 12:32
> >>
> >> Hi Matthias,
> >>
> >> On Thu, 2009-10-29 at 08:53 +0100, Matthias Dieter Wallnöfer wrote:
> >>      
> >>> Regarding the file permissions: ldb sets them per default to 0666. s4
> >>> uses 0600. I really don't see the need to add a default value for
> >>> "set_create_perms" which could also be just 0666 since the method is
> >>> located in the generic LDB bindings. We really need to call it since our
> >>> s4 isn't the default!
> >>>        
> >> I'm not arguing we shouldn't be calling set_create_perms. Python is
> >> different from C in that it supports optional arguments. I'm suggesting
> >> this is a good place to use this feature, to avoid set_create_perms()
> >> from being called twice on a given ldb file if mode 0600 is
> >> indesirable.
> >>
> >> What is the public API for setting the wrap_url useful for exactly? Who
> >> would call it?
> >>
> >> Cheers,
> >>
> >> Jelmer
> >>
> >>      
> >>> Jelmer Vernooij wrote:
> >>>        
> >>>> Hi Matthias,
> >>>>
> >>>> On Mon, Oct 26, 2009 at 11:35:28PM +0100, Matthias Dieter Wallnöfer wrote:
> >>>>
> >>>>          
> >>>>> what do you think about this patch? If it fits I would like to push it soon.
> >>>>>
> >>>>>            
> >>>> Again, please avoid doing multiple different things in a single patch. It makes the patch harder to review and likely to be rejected entirely if one of the
> >>>> items in the patch is wrong.
> >>>>
> >>>> Please avoid mentioning that some functions return None - this is implied if
> >>>> there is no return value type specified.
> >>>>
> >>>> Since Python supports optional arguments, can you perhaps make the mode of
> >>>> the LDB file an optional argument that defaults to 0600 ?
> >>>>
> >>>> What is the public API for setting the wrap_url useful for exactly? Who
> >>>> would call it?
> >>>>
> >>>> Cheers,
> >>>>
> >>>> Jelmer
> >>>>
> >>>>
> >>>>          
> >>>>> From: Matthias Dieter Walln??fer<mwallnoefer at yahoo.de>
> >>>>> Date: Fri, 23 Oct 2009 13:16:54 +0000 (+0200)
> >>>>> Subject: s4:LDB bindings - Work to make the python wrap connection more like the C wrap connection
> >>>>> X-Git-Url: http://repo.or.cz/w/Samba/mdw.git?a=commitdiff_plain;h=5172606b5251078049cb1531abc205fc2af2c17d
> >>>>>
> >>>>> s4:LDB bindings - Work to make the python wrap connection more like the C wrap connection
> >>>>>
> >>>>> - Adds a call for setting the wrap URL on the LDB context
> >>>>> - Add create permissions also on python bindings
> >>>>> - Reorder some function bodies in "pyglue" to match the order in "ldb_wrap_connect"
> >>>>> ---
> >>>>>
> >>>>> diff --git a/source4/scripting/python/pyglue.c b/source4/scripting/python/pyglue.c
> >>>>> index 71203d3..65a23f3 100644
> >>>>> --- a/source4/scripting/python/pyglue.c
> >>>>> +++ b/source4/scripting/python/pyglue.c
> >>>>> @@ -102,6 +102,27 @@ static PyObject *py_set_debug_level(PyObject *self, PyObject *args)
> >>>>>         Py_RETURN_NONE;
> >>>>>     }
> >>>>>
> >>>>> +static PyObject *py_ldb_set_session_info(PyObject *self, PyObject *args)
> >>>>> +{
> >>>>> +    PyObject *py_session_info, *py_ldb;
> >>>>> +    struct auth_session_info *info;
> >>>>> +    struct ldb_context *ldb;
> >>>>> +    if (!PyArg_ParseTuple(args, "OO",&py_ldb,&py_session_info))
> >>>>> +        return NULL;
> >>>>> +
> >>>>> +    PyErr_LDB_OR_RAISE(py_ldb, ldb);
> >>>>> +    /*if (!PyAuthSession_Check(py_session_info)) {
> >>>>> +        PyErr_SetString(PyExc_TypeError, "Expected session info object");
> >>>>> +        return NULL;
> >>>>> +    }*/
> >>>>> +
> >>>>> +    info = PyAuthSession_AsSession(py_session_info);
> >>>>> +
> >>>>> +    ldb_set_opaque(ldb, "sessionInfo", info);
> >>>>> +
> >>>>> +    Py_RETURN_NONE;
> >>>>> +}
> >>>>> +
> >>>>>     static PyObject *py_ldb_set_credentials(PyObject *self, PyObject *args)
> >>>>>     {
> >>>>>         PyObject *py_creds, *py_ldb;
> >>>>> @@ -144,24 +165,17 @@ static PyObject *py_ldb_set_loadparm(PyObject *self, PyObject *args)
> >>>>>         Py_RETURN_NONE;
> >>>>>     }
> >>>>>
> >>>>> -
> >>>>> -static PyObject *py_ldb_set_session_info(PyObject *self, PyObject *args)
> >>>>> +static PyObject *py_ldb_set_wrap_url(PyObject *self, PyObject *args)
> >>>>>     {
> >>>>> -    PyObject *py_session_info, *py_ldb;
> >>>>> -    struct auth_session_info *info;
> >>>>> +    PyObject *py_ldb;
> >>>>> +    char *wrap_url;
> >>>>>         struct ldb_context *ldb;
> >>>>> -    if (!PyArg_ParseTuple(args, "OO",&py_ldb,&py_session_info))
> >>>>> +    if (!PyArg_ParseTuple(args, "Os",&py_ldb,&wrap_url))
> >>>>>             return NULL;
> >>>>>
> >>>>>         PyErr_LDB_OR_RAISE(py_ldb, ldb);
> >>>>> -    /*if (!PyAuthSession_Check(py_session_info)) {
> >>>>> -        PyErr_SetString(PyExc_TypeError, "Expected session info object");
> >>>>> -        return NULL;
> >>>>> -    }*/
> >>>>> -
> >>>>> -    info = PyAuthSession_AsSession(py_session_info);
> >>>>>
> >>>>> -        ldb_set_opaque(ldb, "sessionInfo", info);
> >>>>> +    ldb_set_opaque(ldb, "wrap_url", wrap_url);
> >>>>>
> >>>>>         Py_RETURN_NONE;
> >>>>>     }
> >>>>> @@ -455,15 +469,18 @@ static PyMethodDef py_misc_methods[] = {
> >>>>>             "Generate random password with specified length." },
> >>>>>         { "unix2nttime", (PyCFunction)py_unix2nttime, METH_VARARGS,
> >>>>>             "unix2nttime(timestamp) ->   nttime" },
> >>>>> -    { "ldb_set_credentials", (PyCFunction)py_ldb_set_credentials, METH_VARARGS,
> >>>>> -        "ldb_set_credentials(ldb, credentials) ->   None\n"
> >>>>> -        "Set credentials to use when connecting." },
> >>>>>         { "ldb_set_session_info", (PyCFunction)py_ldb_set_session_info, METH_VARARGS,
> >>>>> -        "ldb_set_session_info(ldb, session_info)\n"
> >>>>> +        "ldb_set_session_info(ldb, session_info) ->   None\n"
> >>>>>             "Set session info to use when connecting." },
> >>>>> +    { "ldb_set_credentials", (PyCFunction)py_ldb_set_credentials, METH_VARARGS,
> >>>>> +        "ldb_set_credentials(ldb, credentials) ->   None\n"
> >>>>> +        "Set credentials to use when connecting." },
> >>>>>         { "ldb_set_loadparm", (PyCFunction)py_ldb_set_loadparm, METH_VARARGS,
> >>>>> -        "ldb_set_loadparm(ldb, session_info)\n"
> >>>>> +        "ldb_set_loadparm(ldb, session_info) ->   None\n"
> >>>>>             "Set loadparm context to use when connecting." },
> >>>>> +    { "ldb_set_wrap_url", (PyCFunction)py_ldb_set_wrap_url, METH_VARARGS,
> >>>>> +        "ldb_set_wrap_url(ldb, wrap_url)\n"
> >>>>> +        "Set wrap URL to use when connecting." },
> >>>>>         { "samdb_set_domain_sid", (PyCFunction)py_samdb_set_domain_sid, METH_VARARGS,
> >>>>>             "samdb_set_domain_sid(samdb, sid)\n"
> >>>>>             "Set SID of domain to use." },
> >>>>> diff --git a/source4/scripting/python/samba/__init__.py b/source4/scripting/python/samba/__init__.py
> >>>>> index 57cefdd..8b6e6fb 100644
> >>>>> --- a/source4/scripting/python/samba/__init__.py
> >>>>> +++ b/source4/scripting/python/samba/__init__.py
> >>>>> @@ -103,18 +103,28 @@ class Ldb(ldb.Ldb):
> >>>>>                 if nosync_p is not None and nosync_p == true:
> >>>>>                     flags |= FLG_NOSYNC
> >>>>>
> >>>>> +        # we usually want Samba databases to be private. If we later find we
> >>>>> +        # need one public, we will have to change this here
> >>>>> +        self.set_create_perms(0600)
> >>>>> +
> >>>>>             if url is not None:
> >>>>>                 self.connect(url, flags, options)
> >>>>>
> >>>>> -    def set_credentials(self, credentials):
> >>>>> -        glue.ldb_set_credentials(self, credentials)
> >>>>> +            # setup for leak detection
> >>>>> +            self.set_wrap_url(url)
> >>>>>
> >>>>>         def set_session_info(self, session_info):
> >>>>>             glue.ldb_set_session_info(self, session_info)
> >>>>>
> >>>>> +    def set_credentials(self, credentials):
> >>>>> +        glue.ldb_set_credentials(self, credentials)
> >>>>> +
> >>>>>         def set_loadparm(self, lp_ctx):
> >>>>>             glue.ldb_set_loadparm(self, lp_ctx)
> >>>>>
> >>>>> +    def set_wrap_url(self, wrap_url):
> >>>>> +        glue.ldb_set_wrap_url(self, wrap_url)
> >>>>> +
> >>>>>         def searchone(self, attribute, basedn=None, expression=None,
> >>>>>                       scope=ldb.SCOPE_BASE):
> >>>>>             """Search for one attribute as a string.
> >>>>>
> >>>>>            
> >>>>
> >>>>          
> >>>        
> >>
> >> -- 
> >> Jelmer Vernooij<jelmer at samba.org>  - http://samba.org/~jelmer/
> >> Jabber: jelmer at jabber.fsfe.org
> >>
> >>
> >>
> >>
> >>      
> >    
> 


-- 
Jelmer Vernooij <jelmer at samba.org> - http://samba.org/~jelmer/
Jabber: jelmer at jabber.fsfe.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20091105/1834ab5e/attachment.pgp>


More information about the samba-technical mailing list