On 08/10/2015 07:04 AM, Lukas Slebodnik wrote:
On (07/08/15 20:54), Michal Židek wrote:
> Hi,
>
> see the attached patches for ticket
>
https://fedorahosted.org/sssd/ticket/2676
>
> Removing the user during cleanup task completely
> removes that user from local database, so that
> user is no longer part of any group in sysdb.
>
> Such user may, or may not have been removed from
> LDAP so the only way to figure out is check
> on the next refresh otherwise we may get
> inconsistent results.
>
> The third patch adds integration test for this
> Apply and run the intgcheck without the first 2
> patches to see the failed test (you will also see that
> all the tests after this failed one will end with
> errors as well, that is probably due to incomplete
> clean up after the ldap tests and I do not intend to
> solve this as part of this patchset but will
> investigate it later).
>
> +struct ldb_dn *sysdb_ldb_val_to_ldb_dn(TALLOC_CTX *tctx,
> + struct sss_domain_info *dom,
> + struct ldb_val *ldbval)
> +{
> + return ldb_dn_from_ldb_val(tctx, dom->sysdb->ldb, ldbval);
> +}
I would prefer to do not expose such simple functions as sysdb functions.
I do not see a reason why should convert ldb_val -> ldb_dn
in other modules.
If wee need two versions of sysdb_mark_entry_as_expired e can still have two
functions:
sysdb_mark_entry_as_expired_ldb_dn
sysdb_mark_entry_as_expired_ldb_val
The core logic can be in one and second will be wrapper with
ldb_dn_from_ldb_val.
I added a wrapper.
> + ret = sss_ldb_modify_permissive(dom->sysdb->ldb,
msg);
We should avoid using sss_ldb_modify_permissive.
It should be used in very rare cases.
Could you explain why do we need to it here?
I wanted to ignore missing entries. But in
the updated patches I simply ignore ENOENT in
the caller and do not use sss_ldb_modify_permissive
anymore.
> +
> + entry_cache_timeout = 1
> + entry_cache_group_timeout = 5000
It would be good either to add comment that
entry_cache_group_timeout should be different to entry_cache_user_timeout
or explicitly use both options in sssd.conf
Ok, I now explicitly set both.
> +
> + time.sleep(10)
This can cause issuse in CI. The first clean up task
is fired after 10 seconds. It would be be better to
wait a little bit longer. To be sure that clean-up
ask already finished.
Changed to 15 seconds. Do you think it is enough?
> +
> + ent.assert_group_by_name(
> + "group2",
> + dict(mem=ent.contains_only("user1")))
LS
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Michal