URL: https://github.com/SSSD/sssd/pull/943 Title: #943: files_ops: Fix cached password remove
alexey-tikhonov commented: """ Hi @elkoniu, Thank you for your patch. Please see comments inline. In regards of `sysdb_delete_recursive_with_filter()` those are mostly nitpicks. There is only one [real concern](https://github.com/SSSD/sssd/pull/943#discussion_r353933715). Changes of `files_ops.c` have some more serious issues. But ultimately my fundamental concerns are as following: 1) files_provider's implementation is already very suboptimal. This patch doubles reading of db files (`enum_files_*()`) thus making it slower. Of course, most probably reading files is not a bottleneck (I guess updating sysdb is). Still I think this could (and better) be avoided even within current approach. 2) What happens if, for example, `uid` of a user was modified? Do we really want to keep password hash (and other custom attributes, if any) in this case? Or do we want to keep custom attributes of unchanged entries only? I am not sure. I would like to have this explained explicitly. In general, from my point of view, proper approach would be to calculate and apply diff only, instead of full sysdb rebuild. This also would be beneficial from performance point of view. But this probably is not best candidate for a one of the very first PRs... """ See the full comment at https://github.com/SSSD/sssd/pull/943#issuecomment-561885990
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org
