6:51 p.m.
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
4:02 a.m.
New subject: [sssd PR#943][comment] files_ops: Fix cached password remove
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
4:13 a.m.
New subject: [sssd PR#943][comment] files_ops: Fix cached password remove
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
4:13 a.m.
New subject: [sssd PR#943][comment] files_ops: Fix cached password remove
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
9:16 a.m.
New subject: [sssd PR#943][comment] files_ops: Fix cached password remove
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
9:16 a.m.
New subject: [sssd PR#943][+Changes requested] files_ops: Fix cached password remove
URL: https://github.com/SSSD/sssd/pull/943
Title: #943: files_ops: Fix cached password remove
Label: +Changes requested
7:16 p.m.
New subject: [sssd PR#943][synchronized] files_ops: Fix cached password remove
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
9:36 p.m.
New subject: [sssd PR#943][comment] files_ops: Fix cached password remove
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
3:01 a.m.
New subject: [sssd PR#943][comment] files_ops: Fix cached password remove
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
5:18 a.m.
New subject: [sssd PR#943][synchronized] files_ops: Fix cached password remove
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
11:03 a.m.
New subject: [sssd PR#943][comment] files_ops: Fix cached password remove
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
5:18 p.m.
New subject: [sssd PR#943][comment] files_ops: Fix cached password remove
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
9:31 p.m.
New subject: [sssd PR#943][synchronized] files_ops: Fix cached password remove
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
9:36 p.m.
New subject: [sssd PR#943][comment] files_ops: Fix cached password remove
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
7:02 a.m.
New subject: [sssd PR#943][+Deferred] [wip] files_ops: Fix cached password remove
URL: https://github.com/SSSD/sssd/pull/943
Title: #943: [wip] files_ops: Fix cached password remove
Label: +Deferred
7:02 a.m.
New subject: [sssd PR#943][comment] [wip] files_ops: Fix cached password remove
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
7:02 a.m.
New subject: [sssd PR#943][closed] [wip] files_ops: Fix cached password remove
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
1301
days inactive
1617
days old
sssd-devel@lists.fedorahosted.org
16 comments
3 participants
participants (3)
-
alexey-tikhonov -
elkoniu
-
pbrezina