Python bindings for keytab manipulation

Jelmer Vernooij jelmer at vernstok.nl
Mon Feb 14 05:26:03 MST 2011


Hi Matthieu,

On Mon, 2011-02-14 at 13:02 +0300, Matthieu Patou wrote:
> Following our talk on IRC you pointed the following points
> 1) why is loadkeytab not a method on Krb5Keytab ?
> 2) please don't copy the SIGINT hack into any other python modules
> 3) __version__  is commonly used for module version strings
> 4) super() takes the current type object as first argument, rather than 
> the parents class object
> 5) the docstring for your test script refers to xattr :)
> 6) the module is named krb5 but is shipped with Samba; I think it'd be 
> nice to have it upstream (so non-Samba users also get the ability to use 
> keytabs from Python) or alternatively moved into the samba module name 
> space (samba.keytab)
> 
> 
> So  for 1), I would say why not, because the api for me would not be so 
> clear if a Keytab object would allow to load a keytab questions like 
> what do we do if there is already entries have to be decided and I think 
> that it can lead to mistake.
Ok. I find it convenient if this sort of method is located on the object
it deals with, but this also doesn't seem too unreasonable.

> 3) version -> __version__ it's fixed I copy it also so the change can be 
> done as well in other modules
I'll fix it in glue, it seems to be the only module that does that.

> 6) Ok so I put it in the samba architecture under the krb5 namespace as 
> potentially there is also keytab for kerberos4, it could be named 
> kerberos5 if we want to avoid confusion with heimdal. I chose to put in 
> the samba namespace as we will need soon more samba function to dig 
> information form the samdb.
Would these have to be part of the "samba.krb5" module though? It
really seems like this module ought to be the "heimdal" Python module,
and upstreamed so other (non-Samba) people can enjoy it too.

> The last version is here: 
> http://git.samba.org/?p=mat/samba.git;a=shortlog;h=refs/heads/misc_review
Some more minor things:

- There is a typo "Keberos" in one of the error messages.
- In docstrings it is convention to use "S.method(arg1, arg2) ->
returntype" rather than "classname.method(arg1, arg2) -> returntype"
- py_krb5_keytab_new doesn't free the python object in the error cases
- If PyArg_ParseTuple just return NULL immediately, that function
already sets an exception
- Consider using PyErr_Format() rather than talloc_asprintf and
PyErr_SetString()
- If you set an exception, *always* return NULL, never None or the
exception won't be noticed by the caller
- In py_krb5_keytab_write you set an exception without returning
- Please check for talloc returning NULL
- If memory allocation is a problem, please raise MemoryError
("PyErr_NoMemory();") rather than a RuntimeError
- It probably makes sense to have a separate exception type for kerberos
errors rather than (ab)using RuntimeError.

Cheers,

jelmer


More information about the samba-technical mailing list