[SSSD] [PATCHES] python bindings for managing users in local domains

Stephen Gallagher sgallagh at redhat.com
Fri Aug 21 13:08:11 UTC 2009


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/20/2009 06:01 PM, Jakub Hrozek wrote:
> Attached are three patches.
> 
> [PATCH 1/3] Refactor tools code
> Refactores tools code while still retaining the original structure.
> Move parameter parsing in tools before attempting to do anything that
> might fail - so that we have debug_level set correctly for potential
> error messages. That allows printing the --help and --usage messages
> without being root.
> 
> [PATCH 2/3] Decouple synchronous sysdb interface from tools
> Instead of working directly with async code in tools, create synchronous
> wrappers that could be used by tools and python bindings.
> 
> [PATCH 3/3] Provide python bindings for sysdb
> Implement a set of python bindings for the sysdb with feature set
> similar to what is available in the tools. The primary
> consumers would be applications like system-config-users.
> 
> These three patches resolve tickets 102, 87 and 95.
> 
> 	Jakub


Patch 0001:
Nack:
domain_lookup(): Please check that ops is not NULL before using it, just
to be safe. Also, the second return ret doesn't make sense. It's not
reset anywhere, so it can only be EOK. I rather assume that's not what
you were going for here. Also, why are you checking if ops->uid is
nonzero in that if() statement? I think just checking
if(ops->domain && ops->domain != dom) is sufficient (since
get_domain_by_id() has already established the validity of ops->uid)
Finally, please return EOK at the end, rather than 0 (for clarity)

run_command(): Please explicitly check if (ret != EXIT_SUCCESS) rather
than just if (ret).


Patch 0002:
Nack:
To perform a safe synchronous transaction, it would be best to guarantee
a private event context, rather than use data->ctx->ev.
synchronous_transaction() should set up a new tevent_context for the
duration of the transaction. (I'm aware that the tools themselves are
more-or-less synchronous, but if we opt to use this interface outside
the tools, we can't guarantee that the event context is safe to use)

parse_groups(): Remove the FIXME about strtok(). We're not using it anymore.

When moving run_command(), please ensure that you make the same explicit
check mentioned in Patch 0001



Patch 0003:
Nack.
I think we probably want the python bindings built by default, with the
option to turn them off. Unless you can make a good argument against it.

In python.m4, you do not properly quote the output strings for
AC_MSG_RESULT and AC_MSG_ERROR

Implementation of the bindings themselves look good to me.


- ------------------------------------------------------------------------

_______________________________________________
sssd-devel mailing list
sssd-devel at lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel


- -- 
Stephen Gallagher
RHCE 804006346421761

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkqOnDYACgkQeiVVYja6o6MD4QCfWbUPNwisPMIwjH9eDQkwif6O
G8UAnRlZIgtO0GbZ+OnQc91nR6MFuOBc
=bsXl
-----END PGP SIGNATURE-----



More information about the sssd-devel mailing list