URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
alexey-tikhonov commented:
"""
> > > `poll_canary` has no meaning as we currently block when refreshing the
> > > cache. We can either remove it or report a bug against sssd that it
> > >
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
alexey-tikhonov commented:
"""
> > > `poll_canary` has no meaning as we currently block when refreshing the
> > > cache. We can either remove it or report a bug against sssd that it
> > >
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
pbrezina commented:
"""
* https://github.com/SSSD/sssd/pull/955 Files provider.
* https://github.com/SSSD/sssd/pull/956 Enumeration.
"""
See the full comment at
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
pbrezina commented:
"""
* https://github.com/SSSD/sssd/pull/956 Files provider.
* https://github.com/SSSD/sssd/pull/956 Enumeration.
"""
See the full comment at
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
pbrezina commented:
"""
> Lets see what other will say. If your patch will not be merged I will try my
> best to investigate issue and explain `INTERACTIVE_TIMEOUT/2`
Fair enough. I will
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
pbrezina commented:
"""
You actually don't even need to call `sss_cache -E` because files provider
invalidates memory cache on its own:
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
pbrezina commented:
"""
> I. `test_files_provider.py`
>
> > `poll_canary` has no meaning as we currently block when refreshing the
> > cache. We can either remove it or report a bug against
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
alexey-tikhonov commented:
"""
> about the race between adding e.g. the user to /etc/passwd and getting
> inotify processed.
>
> Isn't this similar to why calling `sss_cache` was added
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
sumit-bose commented:
"""
Hi,
about the race between adding e.g. the user to /etc/passwd and getting inotify
processed.
Isn't this similar to why calling `sss_cache` was added `useradd`
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
alexey-tikhonov commented:
"""
I. `test_files_provider.py`
> `poll_canary` has no meaning as we currently block when refreshing the cache.
> We can either remove it or report a bug against
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
pbrezina commented:
"""
> I see this as a bug in the SSSD (libnss_files.so doesn't have such an issue).
> And I think hiding this bug in a test is not the best way to deal with it.
Its not a
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
pbrezina commented:
"""
This is exactly the reason why it has not been fixed for two+ years.
@sumit-bose @thalman @mzidek-rh can any of you chime in?
"""
See the full comment at
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
pbrezina commented:
"""
BTW I am not opposing your arguments, it is just not worth the time (and then
again I could have already written a patch instead of commenting). So feel free
to take
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
pbrezina commented:
"""
This is exactly the reason why it has not been fixed for two+ years. You remind
me Lukas so much (in both good and bad way) :-) @sumit-bose @thalman @mzidek-rh
can
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
alexey-tikhonov commented:
"""
First of all:
- I understand importance of "green" CI;
- those patches make CI "green".
So thank you for putting an effort in attempt to resolve this issue
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
pbrezina commented:
"""
Tests works now. If we will see more failures we can always push another patch.
There is no reason to spend more time on this. So I kindly as you to finish
this
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
alexey-tikhonov commented:
"""
Btw, I still think it makes sense to have `INTERACTIVE_TIMEOUT >
ENUMERATION_TIMEOUT + delay`. Even if currently CI runs fine with (equal
timeouts &
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
alexey-tikhonov commented:
"""
Probably it makes sense to split this PR into two different?
"""
See the full comment at
https://github.com/SSSD/sssd/pull/947#issuecomment-560439026
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
pbrezina commented:
"""
There was an infrastructure failure, not related to tests. Running again.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/947#issuecomment-560355996
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
pbrezina commented:
"""
All green. One more time.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/947#issuecomment-560295647
___
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
pbrezina commented:
"""
Works. Let try another run.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/947#issuecomment-559811981
___
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
pbrezina commented:
"""
Ok, lets try it with Fabiano's patch.
"""
See the full comment at
https://github.com/SSSD/sssd/pull/947#issuecomment-559772175
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
alexey-tikhonov commented:
"""
> pbrezina force-pushed the pbrezina:fixtests
Looks like change of the value of this `/2` timeout is what actually fixes
race. And this is exactly the
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
alexey-tikhonov commented:
"""
F29 failed `test_enumeration.py::test_add_remove_membership_rfc2307`...
"""
See the full comment at
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
pbrezina commented:
"""
Thank you both for your time. I amended in-code comments and commit messages.
1) About files provider.
You are right that the sleep is suppose to eliminated race
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
alexey-tikhonov commented:
"""
> @pbrezina, Hi Pavel :)
> I don't really remember, sorry, but looking at the code now it might be to
> avoid racing with SSSD initialization, while making the
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
alexey-tikhonov commented:
"""
> @pbrezina, Hi Pavel :)
> I don't really remember, sorry, but looking at the code now it might be to
> avoid racing with SSSD initialization, while making the
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
spbnick commented:
"""
@pbrezina, Hi Pavel :)
I don't really remember, sorry, but looking at the code now it might be to
avoid racing with SSSD initialization, while making the change before
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
alexey-tikhonov commented:
"""
> > tl,dr: I'm not categorically against estimation of REAL_ENUMERATION_TIMEOUT
> > as 2*ENUMERATION_TIMEOUT.
> > But IMO this has nothing to do with Nyquist
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
alexey-tikhonov commented:
"""
> Sorry if I was not clear enough.
> I was talking about different bug/race: when updated data are requested
> _before_ files_provider even started update.
>
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
pbrezina commented:
"""
> tl,dr: I'm not categorically against estimation of REAL_ENUMERATION_TIMEOUT
> as 2*ENUMERATION_TIMEOUT.
> But IMO this has nothing to do with Nyquist frequency and
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
alexey-tikhonov commented:
"""
> About files provider tests: I checked if there is a bug that you mentioned
> and no. There does not seem to be any bug, though the behavior is not the
>
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
alexey-tikhonov commented:
"""
> Enumeration is run periodically. It is scheduled during SSSD startup and then
> it runs every four seconds. We do not know precisely when the enumeration
>
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
pbrezina commented:
"""
Enumeration is run periodically. It is scheduled during SSSD startup and then
it runs every four seconds. We do not know precisely when the enumeration
refresh is
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
alexey-tikhonov commented:
"""
In regards of `test_enumeration.py`
- I am not sure Nyquist frequency is relevant. I agree that test should wait
longer than enumeration period to allow
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
alexey-tikhonov commented:
"""
In regards of `test_files_provider.py`
There is common pattern like:
```
def check_group(exp_group, delay=3.0):
if delay > 0:
time.sleep(delay)
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
pbrezina commented:
"""
@alexey-tikhonov Can you review this please? I would like to merge it soon so
we have green tests.
"""
See the full comment at
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
pbrezina commented:
"""
All tests passed third time. The failure in CentOS CI is not related: it is
some infrastructure error that started to happen today, see
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
pbrezina commented:
"""
retest this please
"""
See the full comment at
https://github.com/SSSD/sssd/pull/947#issuecomment-559047097
___
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
pbrezina commented:
"""
retest this please
"""
See the full comment at
https://github.com/SSSD/sssd/pull/947#issuecomment-558998434
___
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
pbrezina commented:
"""
Second time succeeded. I altered the commit message little bit and add a
comment to enumeration tests.
"""
See the full comment at
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
pbrezina commented:
"""
First run succeeded. I added some more explanation to the commits and let's see
if we get all green result the seconds time.
"""
See the full comment at
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
pbrezina commented:
"""
I will add some, once I know it fixes the issue (I'm pretty confident about
it). Lets wait for the tests to finish.
"""
See the full comment at
URL: https://github.com/SSSD/sssd/pull/947
Title: #947: tests: fix race conditions in integration tests
alexey-tikhonov commented:
"""
Probably, those changes need better justification taking into account
[3463](https://pagure.io/SSSD/sssd/issue/3463) and #345...
"""
See the full comment at
44 matches
Mail list logo
