URL: https://github.com/SSSD/sssd/pull/274
Title: #274: Merge sss_cache and sss_debuglevel into sssctl
lslebodn commented:
"""
master:
* f74408e37a3007aa41b19ab2afb693a91694da42
* da19eaea902744ec3cb41f87fa93fadb767f90e7
* d2c614143870e6efd4b3ab20c3a55cf714595256
"""
See the full comment at
URL: https://github.com/SSSD/sssd/pull/274
Title: #274: Merge sss_cache and sss_debuglevel into sssctl
justin-stephenson commented:
"""
@lslebodn yes looks good to me, thank you for that.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/274#issuecomment-331957533
URL: https://github.com/SSSD/sssd/pull/274
Title: #274: Merge sss_cache and sss_debuglevel into sssctl
lslebodn commented:
"""
@justin-stephenson, I did few changes to build each commit separately (make git
bisect happy :-)
Are you fine with them?
URL: https://github.com/SSSD/sssd/pull/274
Title: #274: Merge sss_cache and sss_debuglevel into sssctl
justin-stephenson commented:
"""
@lslebodn comments addressed, I was primarily following similar sssctl
functions as an example.
"""
See the full comment at
URL: https://github.com/SSSD/sssd/pull/274
Title: #274: Merge sss_cache and sss_debuglevel into sssctl
justin-stephenson commented:
"""
Thanks for the comments @lslebodn, changes made.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/274#issuecomment-330529794
URL: https://github.com/SSSD/sssd/pull/274
Title: #274: Merge sss_cache and sss_debuglevel into sssctl
lslebodn commented:
"""
few nitpicks: 1st patch "SSSCTL: Replace sss_debuglevel with shell wrapper"
should be after introducing command "sssctl debug*" otherwise it will break
bisect an may
URL: https://github.com/SSSD/sssd/pull/274
Title: #274: Merge sss_cache and sss_debuglevel into sssctl
jhrozek commented:
"""
CI pased: http://vm-058-233.XXX/logs/job/77/62/summary.html
"""
See the full comment at
https://github.com/SSSD/sssd/pull/274#issuecomment-330518395
URL: https://github.com/SSSD/sssd/pull/274
Title: #274: Merge sss_cache and sss_debuglevel into sssctl
justin-stephenson commented:
"""
Thank you for the review @jhrozek
Patchset updated.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/274#issuecomment-330415031
URL: https://github.com/SSSD/sssd/pull/274
Title: #274: Merge sss_cache and sss_debuglevel into sssctl
jhrozek commented:
"""
I'm sorry this PR review stalled (again :-/) but I've read the patches and I
like them now. I have two nitpicks, one was found by Coverity:
Error: CHECKED_RETURN
URL: https://github.com/SSSD/sssd/pull/274
Title: #274: Merge sss_cache and sss_debuglevel into sssctl
justin-stephenson commented:
"""
Patches updated, I believe the Makefile changes are done as Michal proposed but
please let me know if anything needs correcting.
"""
See the full comment at
URL: https://github.com/SSSD/sssd/pull/274
Title: #274: Merge sss_cache and sss_debuglevel into sssctl
justin-stephenson commented:
"""
Thank you for the review @mzidek-rh - I will make the changes and update the PR.
"""
See the full comment at
URL: https://github.com/SSSD/sssd/pull/274
Title: #274: Merge sss_cache and sss_debuglevel into sssctl
mzidek-rh commented:
"""
There is a tabulator instead of space after '=' sign
in Makefile.am:
```
458
459 dist_sbin_SCRIPTS = contrib/tools/sss_debuglevel
460
```
But as Lukas mentioned
URL: https://github.com/SSSD/sssd/pull/274
Title: #274: Merge sss_cache and sss_debuglevel into sssctl
justin-stephenson commented:
"""
PR updated:
- Dropped the sss_cache commits
- Rebased to master
- Added new commit adding `sssctl cache-expire` command as a simple wrapper for
`sss_cache`
URL: https://github.com/SSSD/sssd/pull/274
Title: #274: Merge sss_cache and sss_debuglevel into sssctl
lslebodn commented:
"""
On (20/07/17 10:22), mzidek-rh wrote:
>In sssctl we follow (or at least try to) the TOPIC-ACTION convention for
>naming the commands. So "expire-cache" is not a good
URL: https://github.com/SSSD/sssd/pull/274
Title: #274: Merge sss_cache and sss_debuglevel into sssctl
mzidek-rh commented:
"""
In sssctl we follow (or at least try to) the TOPIC-ACTION convention for naming
the commands. So "expire-cache" is not a good name. It should be
'cache-expire'. We
URL: https://github.com/SSSD/sssd/pull/274
Title: #274: Merge sss_cache and sss_debuglevel into sssctl
lslebodn commented:
"""
Extra message is helpful for users but it still will not fix tests or any other
automated script.
In theory we could merge the package `sssd-tools` into
URL: https://github.com/SSSD/sssd/pull/274
Title: #274: Merge sss_cache and sss_debuglevel into sssctl
justin-stephenson commented:
"""
I am not sure what is the least intrusive way to handle this but moving the
`sss_cache` wrapper to `sssd-tools` makes sense to me. Also, I can add some
URL: https://github.com/SSSD/sssd/pull/274
Title: #274: Merge sss_cache and sss_debuglevel into sssctl
jhrozek commented:
"""
I still see value in having sssctl manage the cache expiration because
especially for sudo rules, we not only need to expire the rules in the cache,
but also tell the
URL: https://github.com/SSSD/sssd/pull/274
Title: #274: Merge sss_cache and sss_debuglevel into sssctl
justin-stephenson commented:
"""
@lslebodn Yes, you make a good point. Is there a suggested way to handle this
or do you prefer to drop the `sss_cache` commits altogether?
"""
See the full
URL: https://github.com/SSSD/sssd/pull/274
Title: #274: Merge sss_cache and sss_debuglevel into sssctl
lslebodn commented:
"""
NACK to sss_cache changes.
sss_cache is part of sssd-common but sssctl is part of sssd-tools.
So after this change shell wrapper `sss_cache` needn't work in some cases
URL: https://github.com/SSSD/sssd/pull/274
Title: #274: Merge sss_cache and sss_debuglevel into sssctl
centos-ci commented:
"""
Can one of the admins verify this patch?
"""
See the full comment at
https://github.com/SSSD/sssd/pull/274#issuecomment-301468941
URL: https://github.com/SSSD/sssd/pull/274
Title: #274: Merge sss_cache and sss_debuglevel into sssctl
centos-ci commented:
"""
Can one of the admins verify this patch?
"""
See the full comment at
https://github.com/SSSD/sssd/pull/274#issuecomment-301468954
22 matches
Mail list logo
