URL: https://github.com/SSSD/sssd/pull/943 Author: elkoniu Title: #943: files_ops: Fix cached password remove Action: opened
PR body: """ When SSSD daemon will detect refresh of password (group) file it will delete all cached users (groups) data. With this change cached data will be deleted only for non existing users (groups).
Resolves: https://pagure.io/SSSD/sssd/issue/3591 """
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/943/head:pr943 git checkout pr943
URL: https://github.com/SSSD/sssd/pull/943 Title: #943: files_ops: Fix cached password remove
alexey-tikhonov commented: """ retest this please """
See the full comment at https://github.com/SSSD/sssd/pull/943#issuecomment-559733674
URL: https://github.com/SSSD/sssd/pull/943 Title: #943: files_ops: Fix cached password remove
alexey-tikhonov commented: """ retest this please """
See the full comment at https://github.com/SSSD/sssd/pull/943#issuecomment-559733674
URL: https://github.com/SSSD/sssd/pull/943 Title: #943: files_ops: Fix cached password remove
alexey-tikhonov commented: """ retest this please """
See the full comment at https://github.com/SSSD/sssd/pull/943#issuecomment-559737650
URL: https://github.com/SSSD/sssd/pull/943 Title: #943: files_ops: Fix cached password remove
alexey-tikhonov commented: """ Hi @elkoniu,
For some reason CI currently fails to compile your branch. I guess rebase is required.
Would you please rebase your PR on current master? """
See the full comment at https://github.com/SSSD/sssd/pull/943#issuecomment-559820590
URL: https://github.com/SSSD/sssd/pull/943 Title: #943: files_ops: Fix cached password remove
Label: +Changes requested
URL: https://github.com/SSSD/sssd/pull/943 Author: elkoniu Title: #943: files_ops: Fix cached password remove Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/943/head:pr943 git checkout pr943
URL: https://github.com/SSSD/sssd/pull/943 Title: #943: files_ops: Fix cached password remove
elkoniu commented: """ Commit rebased to latest master branch. Two functions in the code are unused now. I leave them intentionally as someone may need them in future. """
See the full comment at https://github.com/SSSD/sssd/pull/943#issuecomment-560983823
URL: https://github.com/SSSD/sssd/pull/943 Title: #943: files_ops: Fix cached password remove
pbrezina commented: """ There is no need to keep a dead code. Please, remove them in separate patch. This way it will not be in the code and we can always recover them if needed in the future. """
See the full comment at https://github.com/SSSD/sssd/pull/943#issuecomment-561067604
URL: https://github.com/SSSD/sssd/pull/943 Author: elkoniu Title: #943: files_ops: Fix cached password remove Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/943/head:pr943 git checkout pr943
URL: https://github.com/SSSD/sssd/pull/943 Title: #943: files_ops: Fix cached password remove
alexey-tikhonov commented: """ Hi @elkoniu,
`test_multiple_files_created_after_startup` test fails at every CI machine. I don't recall this test failing previously so probably this is not one of "unstable" tests.
This doesn't necessarily mean there is a problem in the patch. It might be that test needs adjustment (I didn't check).
Could you please check what is the issue with the test? """
See the full comment at https://github.com/SSSD/sssd/pull/943#issuecomment-561262057
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
URL: https://github.com/SSSD/sssd/pull/943 Author: elkoniu Title: #943: files_ops: Fix cached password remove Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/943/head:pr943 git checkout pr943
URL: https://github.com/SSSD/sssd/pull/943 Title: #943: files_ops: Fix cached password remove
elkoniu commented: """ Rebased and minor bugs fixed. Status is still work in progress. """
See the full comment at https://github.com/SSSD/sssd/pull/943#issuecomment-564835434
URL: https://github.com/SSSD/sssd/pull/943 Title: #943: [wip] files_ops: Fix cached password remove
Label: +Deferred
URL: https://github.com/SSSD/sssd/pull/943 Title: #943: [wip] files_ops: Fix cached password remove
alexey-tikhonov commented: """ Closing on agreement with @elkoniu. Might be reopen later. """
See the full comment at https://github.com/SSSD/sssd/pull/943#issuecomment-702087203
URL: https://github.com/SSSD/sssd/pull/943 Author: elkoniu Title: #943: [wip] files_ops: Fix cached password remove Action: closed
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/943/head:pr943 git checkout pr943
sssd-devel@lists.fedorahosted.org