Patch for LDB SAMBA python bindings

Matthias Dieter Wallnöfer mdw at samba.org
Thu Nov 5 08:35:31 MST 2009


Hi Jelmer,

we dropped this part with the wrap URL from the patch. It doesn't seem 
to be needed. I discussed this with abartlet already.

Matthias

Jelmer Vernooij wrote:
> 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
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>          
>>>
>>>        
>>      
>
>    



More information about the samba-technical mailing list