URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
lslebodn commented:
"""
master:
* 1a89fc33d1b9b1070c7ab83fb0314e538ac46736
"""
See the full comment at
https://github.com/SSSD/sssd/pull/259#issuecomment-302104692
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
pbrezina commented:
"""
Ack.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/259#issuecomment-302062567
___
ss
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
sumit-bose commented:
"""
I run a couple of tests with wildcard and certificate searches and all went
well. So ACK from my side.
@pbrezina, do you have any additional c
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
sumit-bose commented:
"""
ah, sorry for the noise, there must have been an older version in my browser
cache.
"""
See the full comment at
https://github.com/SSSD/sssd/
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
fidencio commented:
"""
@sumit-bose: Here's the check:
https://github.com/SSSD/sssd/pull/259/commits/7caf946a72f56e54209be7d130891ee93c7b19dd#diff-153fb0decfbacdbdcfb6b7
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
sumit-bose commented:
"""
> but there's on functional changes in the patch apart from the one suggested
> by yourself
I miss the `if (cr->rctx->get_domains_last_call.t
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
fidencio commented:
"""
CI: http://sssd-ci.duckdns.org/logs/job/69/70/summary.html
"""
See the full comment at
https://github.com/SSSD/sssd/pull/259#issuecomment-301931
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
fidencio commented:
"""
So, I've changed the patch according to @sumit-bose's comments but not
according to @pbrezina's comments (at least not yet).
@pbrezina: I've fai
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
sumit-bose commented:
"""
I withdraw my suggested patch. I think @fidencio's patch is the better fix.
But I would like to ask you to include something like:
dif
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
pbrezina commented:
"""
Open the sockets but postpone reading from them?
"""
See the full comment at
https://github.com/SSSD/sssd/pull/259#issuecomment-300757695
__
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
sumit-bose commented:
"""
jfyi, I think my suggested patch is not the right solution either because it
will delay the startup time especially when the system is offline.
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
pbrezina commented:
"""
Yes, we can go this way as well.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/259#issuecomment-300121646
__
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
fidencio commented:
"""
@sumit-bose, it's okay for me and actually looks cleaner than what I've been
trying to do.
I'd like to hear what @pbrezina thinks about your vers
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
sumit-bose commented:
"""
I would like to suggest an alternative solution for
https://pagure.io/SSSD/sssd/issue/3387.
Since all responders already send a get_domains re
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
pbrezina commented:
"""
`cache_req_update_domains()` can be removed. There is no point for having
function for this. Also I think it will be good if you convert
`cache_
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
fidencio commented:
"""
Just for the record, I've filled https://pagure.io/SSSD/sssd/issue/3390
"""
See the full comment at
https://github.com/SSSD/sssd/pull/259#issuec
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
fidencio commented:
"""
@sumit-bose, @pbrezina: as discussed in an internal meeting, let's try to go
with the version of the patch that we have now and improve it throug
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
pbrezina commented:
"""
An hour timeout is not too long since subdomains rarely change.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/259#issuecomment-2
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
sumit-bose commented:
"""
As I said, if it is done at startup and for unknown domains then checking it
additionally once an hour is fine.
"""
See the full comment at
h
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
fidencio commented:
"""
@pbrezina, @sumit-bose: an hour is not a too long time for this?
"""
See the full comment at
https://github.com/SSSD/sssd/pull/259#issuecomment-
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
sumit-bose commented:
"""
I think it should not only be called unconditionally. It should be called after
startup before any request is processed to be sure we have vali
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
pbrezina commented:
"""
The default timeout is `GET_DOMAINS_DEFAULT_TIMEOUT = 60` seconds, I think an
hour would be good enough. @sumit-bose do you agree?
"""
See the f
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
fidencio commented:
"""
@pbrezina: I'll change it for this version already.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/259#issuecomment-299146482
___
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
pbrezina commented:
"""
I believe this code works. You just don't want to use `tevent_req_post` from
other place then `_send` since it can be use only in a context where
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
fidencio commented:
"""
@sumit-bose: done!
"""
See the full comment at
https://github.com/SSSD/sssd/pull/259#issuecomment-299132388
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
sumit-bose commented:
"""
New version passes all my tests.
Codewise I wonder if the 2 if-blocks in cache_req_process_input() can be
combined in a single one.
"""
See t
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
fidencio commented:
"""
@sumit-bose: patch updated, would you mind giving it a try?
"""
See the full comment at
https://github.com/SSSD/sssd/pull/259#issuecomment-29912
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
sumit-bose commented:
"""
Thank you, that patch is now working great for ListByName request. But it still
fails for ListByCertificate. Can you change the patch that it w
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
fidencio commented:
"""
CI: http://sssd-ci.duckdns.org/logs/job/69/28/summary.html
The fail is not related to this patch.
"""
See the full comment at
https://github.co
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
fidencio commented:
"""
@sumit-bose: patchset has been updated, hopefully it covers all the issues you
had.
I'd also like to hear from @pbrezina on this PR, as it's tou
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
sumit-bose commented:
"""
Thank you, the patch improves the situation (no DBus error returned) and works
reliable for the configured domain.
But if I run a test where S
URL: https://github.com/SSSD/sssd/pull/259
Title: #259: RESPONDER: Also populate cr_domains when initializing the
responders
fidencio commented:
"""
CI: http://sssd-ci.duckdns.org/logs/job/69/03/summary.html
"""
See the full comment at
https://github.com/SSSD/sssd/pull/259#issuecomment-298732
32 matches
Mail list logo
