Re: Shall we use "tenacity" library to help deflake some of Python tests using retry logic?
Thanks for the feedback. I'll prepare a PR to add this library to our test dependencies. There is always a potential that a retry will hide legitimate bugs, and we'd have to make a decision whether a retry is appropriate on a case-by-case basis. The scope of the retry should be as narrow as possible (per test-method, if possible only in certain conditions). I think the key question to ask is: when do we want the failure of this test be actionable? And if the answer is close to "only if it fails all the time", then adding a retry may be better than removing the test altogether. But a better solution is to eliminate flakiness. On Fri, Jan 11, 2019 at 10:54 AM Ahmet Altay wrote: > It makes sense to me too. I share the same concerns as the previous > comments. For any flaky test it would be better to re-write it in a way > that does not cause flakes (e.g. avoid depending on precise timing of > things). If it is not possible, using this library sounds like a good idea > and would reduce overall time we spent of triage flaky tests. > > On Fri, Jan 11, 2019 at 10:43 AM Chamikara Jayalath > wrote: > >> Makes sense in general. However, how do we make sure that this does not >> hide legitimate flakes ? >> >> I think these retries should be applied in a cases by case basis. >> Deflaking the test by updating it is better than a blanket retry. If the >> test is flaky by design and there's no way to update it a blanket retry >> should be OK. Just my 2 cents. >> >> Thanks, >> Cham >> >> >> On Fri, Jan 11, 2019 at 4:30 AM Robert Bradshaw >> wrote: >> >>> I think that makes a lot of sense, and tenacity looks like a decent, >>> maintained library. >>> >>> We should use this sparingly, but it is helpful for algorithms that >>> have an intrinsic amount of randomness/noise (e.g. the sampling code) >>> to reduce a 1% chance of failure to a 1 in a million. >>> >>> On Fri, Jan 11, 2019 at 2:50 AM Valentyn Tymofieiev >>> wrote: >>> > >>> > I have been looking at a few test flakes in Python SDK recently, and >>> some of them can benefit from a simple retry logic. See PR #7455 for an >>> example[1]. >>> > >>> > I would not recommend retrying by default for all tests, or >>> mechanically adding a retry to every test that we see flaking: some >>> legitimate bugs may manifest in flakes that happen rarely, and a retry >>> logic will hide them from us. >>> > >>> > However, in some tests we can consider adding retries, retries with a >>> back-off or retries only on particular exceptions. tenacity[2] offers >>> several decorators for this purpose. It is available under Apache 2.0 >>> license on PyPi [3], and is being maintained. >>> > >>> > What does the community think about adding this library as a test-only >>> dependency to Python SDK? >>> > >>> > Thanks, >>> > Valentyn >>> > >>> > [1] https://github.com/apache/beam/pull/7455 >>> > [2] https://github.com/jd/tenacity >>> > [3] https://pypi.org/project/tenacity/ >>> >>
Re: Shall we use "tenacity" library to help deflake some of Python tests using retry logic?
It makes sense to me too. I share the same concerns as the previous comments. For any flaky test it would be better to re-write it in a way that does not cause flakes (e.g. avoid depending on precise timing of things). If it is not possible, using this library sounds like a good idea and would reduce overall time we spent of triage flaky tests. On Fri, Jan 11, 2019 at 10:43 AM Chamikara Jayalath wrote: > Makes sense in general. However, how do we make sure that this does not > hide legitimate flakes ? > > I think these retries should be applied in a cases by case basis. > Deflaking the test by updating it is better than a blanket retry. If the > test is flaky by design and there's no way to update it a blanket retry > should be OK. Just my 2 cents. > > Thanks, > Cham > > > On Fri, Jan 11, 2019 at 4:30 AM Robert Bradshaw > wrote: > >> I think that makes a lot of sense, and tenacity looks like a decent, >> maintained library. >> >> We should use this sparingly, but it is helpful for algorithms that >> have an intrinsic amount of randomness/noise (e.g. the sampling code) >> to reduce a 1% chance of failure to a 1 in a million. >> >> On Fri, Jan 11, 2019 at 2:50 AM Valentyn Tymofieiev >> wrote: >> > >> > I have been looking at a few test flakes in Python SDK recently, and >> some of them can benefit from a simple retry logic. See PR #7455 for an >> example[1]. >> > >> > I would not recommend retrying by default for all tests, or >> mechanically adding a retry to every test that we see flaking: some >> legitimate bugs may manifest in flakes that happen rarely, and a retry >> logic will hide them from us. >> > >> > However, in some tests we can consider adding retries, retries with a >> back-off or retries only on particular exceptions. tenacity[2] offers >> several decorators for this purpose. It is available under Apache 2.0 >> license on PyPi [3], and is being maintained. >> > >> > What does the community think about adding this library as a test-only >> dependency to Python SDK? >> > >> > Thanks, >> > Valentyn >> > >> > [1] https://github.com/apache/beam/pull/7455 >> > [2] https://github.com/jd/tenacity >> > [3] https://pypi.org/project/tenacity/ >> >
Re: Shall we use "tenacity" library to help deflake some of Python tests using retry logic?
Makes sense in general. However, how do we make sure that this does not hide legitimate flakes ? I think these retries should be applied in a cases by case basis. Deflaking the test by updating it is better than a blanket retry. If the test is flaky by design and there's no way to update it a blanket retry should be OK. Just my 2 cents. Thanks, Cham On Fri, Jan 11, 2019 at 4:30 AM Robert Bradshaw wrote: > I think that makes a lot of sense, and tenacity looks like a decent, > maintained library. > > We should use this sparingly, but it is helpful for algorithms that > have an intrinsic amount of randomness/noise (e.g. the sampling code) > to reduce a 1% chance of failure to a 1 in a million. > > On Fri, Jan 11, 2019 at 2:50 AM Valentyn Tymofieiev > wrote: > > > > I have been looking at a few test flakes in Python SDK recently, and > some of them can benefit from a simple retry logic. See PR #7455 for an > example[1]. > > > > I would not recommend retrying by default for all tests, or mechanically > adding a retry to every test that we see flaking: some legitimate bugs may > manifest in flakes that happen rarely, and a retry logic will hide them > from us. > > > > However, in some tests we can consider adding retries, retries with a > back-off or retries only on particular exceptions. tenacity[2] offers > several decorators for this purpose. It is available under Apache 2.0 > license on PyPi [3], and is being maintained. > > > > What does the community think about adding this library as a test-only > dependency to Python SDK? > > > > Thanks, > > Valentyn > > > > [1] https://github.com/apache/beam/pull/7455 > > [2] https://github.com/jd/tenacity > > [3] https://pypi.org/project/tenacity/ >
Re: Shall we use "tenacity" library to help deflake some of Python tests using retry logic?
I think that makes a lot of sense, and tenacity looks like a decent, maintained library. We should use this sparingly, but it is helpful for algorithms that have an intrinsic amount of randomness/noise (e.g. the sampling code) to reduce a 1% chance of failure to a 1 in a million. On Fri, Jan 11, 2019 at 2:50 AM Valentyn Tymofieiev wrote: > > I have been looking at a few test flakes in Python SDK recently, and some of > them can benefit from a simple retry logic. See PR #7455 for an example[1]. > > I would not recommend retrying by default for all tests, or mechanically > adding a retry to every test that we see flaking: some legitimate bugs may > manifest in flakes that happen rarely, and a retry logic will hide them from > us. > > However, in some tests we can consider adding retries, retries with a > back-off or retries only on particular exceptions. tenacity[2] offers several > decorators for this purpose. It is available under Apache 2.0 license on PyPi > [3], and is being maintained. > > What does the community think about adding this library as a test-only > dependency to Python SDK? > > Thanks, > Valentyn > > [1] https://github.com/apache/beam/pull/7455 > [2] https://github.com/jd/tenacity > [3] https://pypi.org/project/tenacity/