Re: [gentoo-portage-dev] [PATCH 0/4] rsync: add key refresh retry (bug 649276)

[Alec Warner]

On Sun, Apr 1, 2018 at 2:29 PM, Michał Górny  wrote:

> W dniu nie, 01.04.2018 o godzinie 08∶59 -0700, użytkownik Zac Medico
> napisał:
> > On 04/01/2018 03:57 AM, Michał Górny wrote:
> > > W dniu sob, 31.03.2018 o godzinie 19∶46 -0700, użytkownik Zac Medico
> > > napisał:
> > > > Since key refresh is prone to failure, retry using exponential
> > > > backoff with random jitter. This adds the following sync-openpgp-*
> > > > configuration settings:
> > > >
> > > > sync-openpgp-key-refresh-retry-count = 40
> > > >
> > > >   Maximum number of times to retry key refresh if it fails.  Between
> > > >   each  key  refresh attempt, there is an exponential delay with a
> > > >   constant multiplier and a uniform random multiplier between 0 and
> 1.
> > > >
> > > > sync-openpgp-key-refresh-retry-delay-exp-base = 2
> > > >
> > > >   The base of the exponential expression. The exponent is the number
> > > >   of previous refresh attempts.
> > > >
> > > > sync-openpgp-key-refresh-retry-delay-max = 60
> > > >
> > > >   Maximum  delay between each retry attempt, in units of seconds.
> This
> > > >   places a limit on the length of the exponential delay.
> > > >
> > > > sync-openpgp-key-refresh-retry-delay-mult = 4
> > > >
> > > >   Multiplier for the exponential delay.
> > > >
> > > > sync-openpgp-key-refresh-retry-overall-timeout = 1200
> > > >
> > > >   Combined time limit for all refresh attempts, in units of seconds.
> > > >
> > > > Bug: https://bugs.gentoo.org/649276
> > > >
> > > > Zac Medico (4):
> > > >   Add ForkExecutor (bug 649588)
> > > >   Add ExponentialBackoff and RandomExponentialBackoff
> > > >   Add retry decorator (API inspired by tenacity)
> > > >   rsync: add key refresh retry (bug 649276)
> > > >
> > > >  cnf/repos.conf|   5 +
> > > >  man/portage.5 |  19 +++
> > > >  pym/portage/repository/config.py  |  22 
> > > >  pym/portage/sync/modules/rsync/rsync.py   |  16 ++-
> > > >  pym/portage/sync/syncbase.py  |  85 +++-
> > > >  pym/portage/tests/util/futures/test_retry.py  | 147
> +
> > > >  pym/portage/util/_eventloop/EventLoop.py  |  45 ++-
> > > >  pym/portage/util/backoff.py   |  48 +++
> > > >  pym/portage/util/futures/executor/__init__.py |   0
> > > >  pym/portage/util/futures/executor/fork.py | 130
> +++
> > > >  pym/portage/util/futures/futures.py   |   6 +
> > > >  pym/portage/util/futures/retry.py | 178
> ++
> > > >  12 files changed, 697 insertions(+), 4 deletions(-)
> > > >  create mode 100644 pym/portage/tests/util/futures/test_retry.py
> > > >  create mode 100644 pym/portage/util/backoff.py
> > > >  create mode 100644 pym/portage/util/futures/executor/__init__.py
> > > >  create mode 100644 pym/portage/util/futures/executor/fork.py
> > > >  create mode 100644 pym/portage/util/futures/retry.py
> > > >
> > >
> > > This essentially looks like ~700 lines of code to try to workaround
> > > broken networking. I would rather try to do that using 5 lines of code
> > > but that's just me, and my programs aren't enterprise quality. I just
> > > hope it actually solves as many problems as it's going to introduce.
> >
> > The vast majority of this code is generic and reusable, and I do intend
> > to reuse it. For example, the executor support will be an essential
> > piece for the asyncio.AbstractEventLoop for bug 649588.
>
> Sure it is and sure you will. But tell me: who is going to maintain it
> all? Because as far as I can see, we're still dealing with a bus factor
> of one and all you're doing is making it worse. More code, more
> complexity, more creeping featurism and hacks.
>

I'll split this reply into two sorts of points. One is a brief point about
this patchset. The other is a larger point about the project and its
direction.

About this patchset:

I don't mind this code because its not domain specific and I can read it
and understand it. Many apps need async ways to run code, and anyone who
has used the futures API will find this code familiar. I reviewed it in
about 10 minutes. I'm not heavily invested with the async-wagon that Zac is
driving, but I also see how its an area where performance can be improved
by doing more stuff at once using an async operations. I think if you have
more generic concerns with the async frameworks I'd love to see some
discussion about them (particularly around how we can improve perf by using
tighter algorithms; often the 'perf' boost is just higher CPU utilization
and driving multi-core systems which results in better walltime perf, but
not necessarily cpu-perf.)

Its fine to disagree here. I think the project has some requirements around
minimized dependencies, so the choice is either to pull in some kind of
retry-lib and add a dep, or add this code. Its also not immediately clear
if the 3rd party deps would still not 

Re: [gentoo-portage-dev] [PATCH 0/4] rsync: add key refresh retry (bug 649276)

[Zac Medico]

On 04/01/2018 11:29 AM, Michał Górny wrote:
> W dniu nie, 01.04.2018 o godzinie 08∶59 -0700, użytkownik Zac Medico
> napisał:
>> On 04/01/2018 03:57 AM, Michał Górny wrote:
>>> W dniu sob, 31.03.2018 o godzinie 19∶46 -0700, użytkownik Zac Medico
>>> napisał:
 Since key refresh is prone to failure, retry using exponential
 backoff with random jitter. This adds the following sync-openpgp-*
 configuration settings:

 sync-openpgp-key-refresh-retry-count = 40

   Maximum number of times to retry key refresh if it fails.  Between
   each  key  refresh attempt, there is an exponential delay with a
   constant multiplier and a uniform random multiplier between 0 and 1.

 sync-openpgp-key-refresh-retry-delay-exp-base = 2

   The base of the exponential expression. The exponent is the number
   of previous refresh attempts.

 sync-openpgp-key-refresh-retry-delay-max = 60

   Maximum  delay between each retry attempt, in units of seconds. This
   places a limit on the length of the exponential delay.

 sync-openpgp-key-refresh-retry-delay-mult = 4

   Multiplier for the exponential delay.

 sync-openpgp-key-refresh-retry-overall-timeout = 1200

   Combined time limit for all refresh attempts, in units of seconds.

 Bug: https://bugs.gentoo.org/649276

 Zac Medico (4):
   Add ForkExecutor (bug 649588)
   Add ExponentialBackoff and RandomExponentialBackoff
   Add retry decorator (API inspired by tenacity)
   rsync: add key refresh retry (bug 649276)

  cnf/repos.conf|   5 +
  man/portage.5 |  19 +++
  pym/portage/repository/config.py  |  22 
  pym/portage/sync/modules/rsync/rsync.py   |  16 ++-
  pym/portage/sync/syncbase.py  |  85 +++-
  pym/portage/tests/util/futures/test_retry.py  | 147 +
  pym/portage/util/_eventloop/EventLoop.py  |  45 ++-
  pym/portage/util/backoff.py   |  48 +++
  pym/portage/util/futures/executor/__init__.py |   0
  pym/portage/util/futures/executor/fork.py | 130 +++
  pym/portage/util/futures/futures.py   |   6 +
  pym/portage/util/futures/retry.py | 178 
 ++
  12 files changed, 697 insertions(+), 4 deletions(-)
  create mode 100644 pym/portage/tests/util/futures/test_retry.py
  create mode 100644 pym/portage/util/backoff.py
  create mode 100644 pym/portage/util/futures/executor/__init__.py
  create mode 100644 pym/portage/util/futures/executor/fork.py
  create mode 100644 pym/portage/util/futures/retry.py

>>>
>>> This essentially looks like ~700 lines of code to try to workaround
>>> broken networking. I would rather try to do that using 5 lines of code
>>> but that's just me, and my programs aren't enterprise quality. I just
>>> hope it actually solves as many problems as it's going to introduce.
>>
>> The vast majority of this code is generic and reusable, and I do intend
>> to reuse it. For example, the executor support will be an essential
>> piece for the asyncio.AbstractEventLoop for bug 649588.
> 
> Sure it is and sure you will. But tell me: who is going to maintain it
> all? Because as far as I can see, we're still dealing with a bus factor
> of one and all you're doing is making it worse. More code, more
> complexity, more creeping featurism and hacks.

In the worst case, people can disable the retry feature and all of the
associated code can be disabled simply by setting
sync-openpgp-key-refresh-retry-count = 0 in repos.conf.

In order to reduce the bus factor, I'm modeling the code on the standard
library's asyncio framework. This allows use to leverage people's
experience with asyncio, and also introduces people to asyncio concepts
if they are not yet familar.

> Last time you went away, you left us with a horribly unmaintainable
> package manager full of complexity, hacks and creeping featurism
> and a Portage team whose members had barely any knowledge of the code.
> Just when things started moving again, you came back and we're back to
> square one.

I'd love to help reduce the bus factor, and I'm working to do that, like
with the asyncio stuff.

> Today Portage once again is a one-developer project, full of more
> complexity, more hacks and more creeping featurism. And we once again
> have a bus factor of one -- one developer who apparently knows
> everything, does everything and tries to be nice to everyone, except he
> really ignores others, makes a lot of empty promises and consistently
> makes the health of the project go from bad to worse.

I disagree with your synopsis.

> So, please tell me: what happens when you leave again? How have you used
> your time in the project? What have you done to make sure that
> the project stays 

Re: [gentoo-portage-dev] [PATCH 0/4] rsync: add key refresh retry (bug 649276)

[Fabian Groffen]

On 01-04-2018 20:29:59 +0200, Michał Górny wrote:
> > > This essentially looks like ~700 lines of code to try to workaround
> > > broken networking. I would rather try to do that using 5 lines of code
> > > but that's just me, and my programs aren't enterprise quality. I just
> > > hope it actually solves as many problems as it's going to introduce.
> > 
> > The vast majority of this code is generic and reusable, and I do intend
> > to reuse it. For example, the executor support will be an essential
> > piece for the asyncio.AbstractEventLoop for bug 649588.
> 
> Sure it is and sure you will. But tell me: who is going to maintain it
> all? Because as far as I can see, we're still dealing with a bus factor
> of one and all you're doing is making it worse. More code, more
> complexity, more creeping featurism and hacks.

Well, well, well.  This could be said about a lot of code, including
your own.

> Last time you went away, you left us with a horribly unmaintainable
> package manager full of complexity, hacks and creeping featurism
> and a Portage team whose members had barely any knowledge of the code.
> Just when things started moving again, you came back and we're back to
> square one.

I don't see why this has to be made personal.  In the olde days people
just pushed whatever they needed, now it's reviewed and acked, so how
can it be back to square one?

> Today Portage once again is a one-developer project, full of more
> complexity, more hacks and more creeping featurism. And we once again
> have a bus factor of one -- one developer who apparently knows
> everything, does everything and tries to be nice to everyone, except he
> really ignores others, makes a lot of empty promises and consistently
> makes the health of the project go from bad to worse.

Errr, didn't you recently start your own fork with creeping featurism of
its own because mainline was/is too slow?

> So, please tell me: what happens when you leave again? How have you used
> your time in the project? What have you done to make sure that
> the project stays alive without you? Because as far as I can see, adding
> few thousand of lines of practically unreviewed code every month does
> not help with that.

Why is this Zac's problem specifically?  Isn't this a general problem of
Gentoo?  And isn't your attitude contributing in a large factor to
the bus-factor getting down from 1 to 0?

> I forked Portage because I didn't want to fight with you. When I forked
> it, I declared that I will merge mainline changes regularly for
> the benefit of my users. But after a week, I really start feeling like
> that's going to be a really bad idea. Like it's time to forget about
> mainline Portage as a completely dead end, and go our separate ways.

So you fork.  Like many forks, that is because people feel that it isn't
going "their way".  Yay.  That doesn't make you "right".

> I'm seriously worried about the future of Gentoo. I'd really appreciate
> if you started focusing on that as well. I get that all this stuff looks
> cool on paper but few months or years from now, someone will curse
> 'whoever wrote that code' while trying to fix some nasty bug. Or get
> things moving forward. Or implement EAPI 8.

Perhaps it's time to look into the mirror.

Thanks,
Fabian

-- 
Fabian Groffen
Gentoo on a different level


signature.asc
Description: PGP signature

Re: [gentoo-portage-dev] [PATCH 0/4] rsync: add key refresh retry (bug 649276)

[Michał Górny]

W dniu nie, 01.04.2018 o godzinie 08∶59 -0700, użytkownik Zac Medico
napisał:
> On 04/01/2018 03:57 AM, Michał Górny wrote:
> > W dniu sob, 31.03.2018 o godzinie 19∶46 -0700, użytkownik Zac Medico
> > napisał:
> > > Since key refresh is prone to failure, retry using exponential
> > > backoff with random jitter. This adds the following sync-openpgp-*
> > > configuration settings:
> > > 
> > > sync-openpgp-key-refresh-retry-count = 40
> > > 
> > >   Maximum number of times to retry key refresh if it fails.  Between
> > >   each  key  refresh attempt, there is an exponential delay with a
> > >   constant multiplier and a uniform random multiplier between 0 and 1.
> > > 
> > > sync-openpgp-key-refresh-retry-delay-exp-base = 2
> > > 
> > >   The base of the exponential expression. The exponent is the number
> > >   of previous refresh attempts.
> > > 
> > > sync-openpgp-key-refresh-retry-delay-max = 60
> > > 
> > >   Maximum  delay between each retry attempt, in units of seconds. This
> > >   places a limit on the length of the exponential delay.
> > > 
> > > sync-openpgp-key-refresh-retry-delay-mult = 4
> > > 
> > >   Multiplier for the exponential delay.
> > > 
> > > sync-openpgp-key-refresh-retry-overall-timeout = 1200
> > > 
> > >   Combined time limit for all refresh attempts, in units of seconds.
> > > 
> > > Bug: https://bugs.gentoo.org/649276
> > > 
> > > Zac Medico (4):
> > >   Add ForkExecutor (bug 649588)
> > >   Add ExponentialBackoff and RandomExponentialBackoff
> > >   Add retry decorator (API inspired by tenacity)
> > >   rsync: add key refresh retry (bug 649276)
> > > 
> > >  cnf/repos.conf|   5 +
> > >  man/portage.5 |  19 +++
> > >  pym/portage/repository/config.py  |  22 
> > >  pym/portage/sync/modules/rsync/rsync.py   |  16 ++-
> > >  pym/portage/sync/syncbase.py  |  85 +++-
> > >  pym/portage/tests/util/futures/test_retry.py  | 147 +
> > >  pym/portage/util/_eventloop/EventLoop.py  |  45 ++-
> > >  pym/portage/util/backoff.py   |  48 +++
> > >  pym/portage/util/futures/executor/__init__.py |   0
> > >  pym/portage/util/futures/executor/fork.py | 130 +++
> > >  pym/portage/util/futures/futures.py   |   6 +
> > >  pym/portage/util/futures/retry.py | 178 
> > > ++
> > >  12 files changed, 697 insertions(+), 4 deletions(-)
> > >  create mode 100644 pym/portage/tests/util/futures/test_retry.py
> > >  create mode 100644 pym/portage/util/backoff.py
> > >  create mode 100644 pym/portage/util/futures/executor/__init__.py
> > >  create mode 100644 pym/portage/util/futures/executor/fork.py
> > >  create mode 100644 pym/portage/util/futures/retry.py
> > > 
> > 
> > This essentially looks like ~700 lines of code to try to workaround
> > broken networking. I would rather try to do that using 5 lines of code
> > but that's just me, and my programs aren't enterprise quality. I just
> > hope it actually solves as many problems as it's going to introduce.
> 
> The vast majority of this code is generic and reusable, and I do intend
> to reuse it. For example, the executor support will be an essential
> piece for the asyncio.AbstractEventLoop for bug 649588.

Sure it is and sure you will. But tell me: who is going to maintain it
all? Because as far as I can see, we're still dealing with a bus factor
of one and all you're doing is making it worse. More code, more
complexity, more creeping featurism and hacks.

Last time you went away, you left us with a horribly unmaintainable
package manager full of complexity, hacks and creeping featurism
and a Portage team whose members had barely any knowledge of the code.
Just when things started moving again, you came back and we're back to
square one.

Today Portage once again is a one-developer project, full of more
complexity, more hacks and more creeping featurism. And we once again
have a bus factor of one -- one developer who apparently knows
everything, does everything and tries to be nice to everyone, except he
really ignores others, makes a lot of empty promises and consistently
makes the health of the project go from bad to worse.

So, please tell me: what happens when you leave again? How have you used
your time in the project? What have you done to make sure that
the project stays alive without you? Because as far as I can see, adding
few thousand of lines of practically unreviewed code every month does
not help with that.

I forked Portage because I didn't want to fight with you. When I forked
it, I declared that I will merge mainline changes regularly for
the benefit of my users. But after a week, I really start feeling like
that's going to be a really bad idea. Like it's time to forget about
mainline Portage as a completely dead end, and go our separate ways.

I'm seriously worried about the future of Gentoo. I'd really appreciate
if you 

Re: [gentoo-portage-dev] [PATCH] INSTALL_MASK: honor install time config for binary packages (bug 651952)

[Zac Medico]

On 04/01/2018 06:54 AM, Michał Górny wrote:
> W dniu czw, 29.03.2018 o godzinie 15∶34 -0700, użytkownik Zac Medico
> napisał:
>> For binary packages, honor the INSTALL_MASK configuration that
>> exists at install time, since it might differ from the build time
>> setting.
>>
>> Fixes: 3416876c0ee7 ("{,PKG_}INSTALL_MASK: python implementation")
>> Bug: https://bugs.gentoo.org/651952
>> ---
>>  bin/misc-functions.sh| 23 +++
>>  bin/phase-functions.sh   | 10 +-
>>  pym/portage/dbapi/vartree.py |  5 +
>>  3 files changed, 29 insertions(+), 9 deletions(-)
>>
>> diff --git a/bin/misc-functions.sh b/bin/misc-functions.sh
>> index 26f589915..a6330ee93 100755
>> --- a/bin/misc-functions.sh
>> +++ b/bin/misc-functions.sh
>> @@ -323,6 +323,29 @@ postinst_qa_check() {
>>  done < <(printf "%s\0" "${qa_checks[@]}" | LC_ALL=C sort -u -z)
>>  }
>>  
>> +preinst_mask() {
>> +# Remove man pages, info pages, docs if requested. This is
>> +# implemented in bash in order to respect INSTALL_MASK settings
>> +# from bashrc.
>> +local f x
>> +for f in man info doc; do
>> +if has no${f} ${FEATURES}; then
>> +INSTALL_MASK+=" /usr/share/${f}"
>> +fi
>> +done
>> +
>> +# Store modified variables in build-info.
>> +cd "${PORTAGE_BUILDDIR}"/build-info || die
>> +set -f
>> +
>> +IFS=$' \t\n\r'
>> +for f in INSTALL_MASK; do
> 
> This loop along with the whole indirection is entirely pointless, given
> that you're processing exactly one variable.

I did this for consistency with the related loop in dyn_install, since
we might save values of other variables from the binary package
environment. In fact, I think we should record DOC_SYMLINKS_DIR here,
since it affects CONTENTS, much like INSTALL_MASK.

>> +x=$(echo -n ${!f})
>> +[[ -n ${x} ]] && echo "${x}" > "${f}"
> 
> There's probably no point in this [[ -n ... ]], as that:
> 
> a. requires you to special-handle missing INSTALL_MASK file, while it's
> easier to just ensure that it's there (and I think you requested
> the same thing from me before you rewritten my commit into breakage),
> 
> b. makes it impossible to distinguish packages from before INSTALL_MASK
> storing was added from those where it is empty.

Maybe this only matters within the context of bug 364633 [1], and for
that I think we need to introduce a separate CONTENTS.INSTALL_MASK file
so that we can easily toggle collision-protect behavior to use
CONTENTS.INSTALL_MASK when desired.

[1] https://bugs.gentoo.org/364633
-- 
Thanks,
Zac



signature.asc
Description: OpenPGP digital signature

Re: [gentoo-portage-dev] [PATCH 0/4] rsync: add key refresh retry (bug 649276)

[Zac Medico]

On 04/01/2018 03:57 AM, Michał Górny wrote:
> W dniu sob, 31.03.2018 o godzinie 19∶46 -0700, użytkownik Zac Medico
> napisał:
>> Since key refresh is prone to failure, retry using exponential
>> backoff with random jitter. This adds the following sync-openpgp-*
>> configuration settings:
>>
>> sync-openpgp-key-refresh-retry-count = 40
>>
>>   Maximum number of times to retry key refresh if it fails.  Between
>>   each  key  refresh attempt, there is an exponential delay with a
>>   constant multiplier and a uniform random multiplier between 0 and 1.
>>
>> sync-openpgp-key-refresh-retry-delay-exp-base = 2
>>
>>   The base of the exponential expression. The exponent is the number
>>   of previous refresh attempts.
>>
>> sync-openpgp-key-refresh-retry-delay-max = 60
>>
>>   Maximum  delay between each retry attempt, in units of seconds. This
>>   places a limit on the length of the exponential delay.
>>
>> sync-openpgp-key-refresh-retry-delay-mult = 4
>>
>>   Multiplier for the exponential delay.
>>
>> sync-openpgp-key-refresh-retry-overall-timeout = 1200
>>
>>   Combined time limit for all refresh attempts, in units of seconds.
>>
>> Bug: https://bugs.gentoo.org/649276
>>
>> Zac Medico (4):
>>   Add ForkExecutor (bug 649588)
>>   Add ExponentialBackoff and RandomExponentialBackoff
>>   Add retry decorator (API inspired by tenacity)
>>   rsync: add key refresh retry (bug 649276)
>>
>>  cnf/repos.conf|   5 +
>>  man/portage.5 |  19 +++
>>  pym/portage/repository/config.py  |  22 
>>  pym/portage/sync/modules/rsync/rsync.py   |  16 ++-
>>  pym/portage/sync/syncbase.py  |  85 +++-
>>  pym/portage/tests/util/futures/test_retry.py  | 147 +
>>  pym/portage/util/_eventloop/EventLoop.py  |  45 ++-
>>  pym/portage/util/backoff.py   |  48 +++
>>  pym/portage/util/futures/executor/__init__.py |   0
>>  pym/portage/util/futures/executor/fork.py | 130 +++
>>  pym/portage/util/futures/futures.py   |   6 +
>>  pym/portage/util/futures/retry.py | 178 
>> ++
>>  12 files changed, 697 insertions(+), 4 deletions(-)
>>  create mode 100644 pym/portage/tests/util/futures/test_retry.py
>>  create mode 100644 pym/portage/util/backoff.py
>>  create mode 100644 pym/portage/util/futures/executor/__init__.py
>>  create mode 100644 pym/portage/util/futures/executor/fork.py
>>  create mode 100644 pym/portage/util/futures/retry.py
>>
> 
> This essentially looks like ~700 lines of code to try to workaround
> broken networking. I would rather try to do that using 5 lines of code
> but that's just me, and my programs aren't enterprise quality. I just
> hope it actually solves as many problems as it's going to introduce.

The vast majority of this code is generic and reusable, and I do intend
to reuse it. For example, the executor support will be an essential
piece for the asyncio.AbstractEventLoop for bug 649588.
-- 
Thanks,
Zac



signature.asc
Description: OpenPGP digital signature

Re: [gentoo-portage-dev] [PATCH 3/4] Add retry decorator (API inspired by tenacity)

[Alec Warner]

also lgtm.

On Sat, Mar 31, 2018 at 10:46 PM, Zac Medico  wrote:

> This decorator will be useful for retrying asynchronous
> operations, such as gpg key refresh (bug 649276). The
> API is inspired by tenacity, but is simpler. Only
> asynchronous functions (like @asyncio.coroutine functions)
> are supported. In order to retry a synchronous function,
> first convert it to an asynchronous function as follows:
>
> asynchronous_func = functools.partial(
> loop.run_in_executor, None, synchronous_func)
>
> Bug: https://bugs.gentoo.org/649276
> See: https://github.com/jd/tenacity
> ---
>  pym/portage/tests/util/futures/test_retry.py | 147 ++
>  pym/portage/util/futures/futures.py  |   6 +
>  pym/portage/util/futures/retry.py| 178
> +++
>  3 files changed, 331 insertions(+)
>  create mode 100644 pym/portage/tests/util/futures/test_retry.py
>  create mode 100644 pym/portage/util/futures/retry.py
>
> diff --git a/pym/portage/tests/util/futures/test_retry.py
> b/pym/portage/tests/util/futures/test_retry.py
> new file mode 100644
> index 0..7641e4e92
> --- /dev/null
> +++ b/pym/portage/tests/util/futures/test_retry.py
> @@ -0,0 +1,147 @@
> +# Copyright 2018 Gentoo Foundation
> +# Distributed under the terms of the GNU General Public License v2
> +
> +import functools
> +
> +try:
> +   import threading
> +except ImportError:
> +   import dummy_threading as threading
> +
> +from portage.tests import TestCase
> +from portage.util._eventloop.global_event_loop import global_event_loop
> +from portage.util.backoff import RandomExponentialBackoff
> +from portage.util.futures.futures import TimeoutError
> +from portage.util.futures.retry import retry
> +from portage.util.futures.wait import wait
> +from portage.util.monotonic import monotonic
> +
> +
> +class SucceedLaterException(Exception):
> +   pass
> +
> +
> +class SucceedLater(object):
> +   """
> +   A callable object that succeeds some duration of time has passed.
> +   """
> +   def __init__(self, duration):
> +   self._succeed_time = monotonic() + duration
> +
> +   def __call__(self):
> +   remaining = self._succeed_time - monotonic()
> +   if remaining > 0:
> +   raise SucceedLaterException('time until success:
> {} seconds'.format(remaining))
> +   return 'success'
> +
> +
> +class SucceedNeverException(Exception):
> +   pass
> +
> +
> +class SucceedNever(object):
> +   """
> +   A callable object that never succeeds.
> +   """
> +   def __call__(self):
> +   raise SucceedNeverException('expected failure')
> +
> +
> +class HangForever(object):
> +   """
> +   A callable object that sleeps forever.
> +   """
> +   def __call__(self):
> +   threading.Event().wait()
> +
> +
> +class RetryTestCase(TestCase):
> +   def testSucceedLater(self):
> +   loop = global_event_loop()
> +   func = SucceedLater(1)
> +   func_coroutine = functools.partial(loop.run_in_executor,
> None, func)
> +   decorator = retry(try_max=,
> +   delay_func=RandomExponentialBackoff(multiplier=0.1,
> base=2))
> +   decorated_func = decorator(func_coroutine)
> +   result = loop.run_until_complete(decorated_func())
> +   self.assertEqual(result, 'success')
> +
> +   def testSucceedNever(self):
> +   loop = global_event_loop()
> +   func = SucceedNever()
> +   func_coroutine = functools.partial(loop.run_in_executor,
> None, func)
> +   decorator = retry(try_max=4, try_timeout=None,
> +   delay_func=RandomExponentialBackoff(multiplier=0.1,
> base=2))
> +   decorated_func = decorator(func_coroutine)
> +   done, pending = loop.run_until_complete(wait([
> decorated_func()]))
> +   self.assertEqual(len(done), 1)
> +   self.assertTrue(isinstance(done[0].exception().__cause__,
> SucceedNeverException))
> +
> +   def testSucceedNeverReraise(self):
> +   loop = global_event_loop()
> +   func = SucceedNever()
> +   func_coroutine = functools.partial(loop.run_in_executor,
> None, func)
> +   decorator = retry(reraise=True, try_max=4,
> try_timeout=None,
> +   delay_func=RandomExponentialBackoff(multiplier=0.1,
> base=2))
> +   decorated_func = decorator(func_coroutine)
> +   done, pending = loop.run_until_complete(wait([
> decorated_func()]))
> +   self.assertEqual(len(done), 1)
> +   self.assertTrue(isinstance(done[0].exception(),
> SucceedNeverException))
> +
> +   def testHangForever(self):
> +   loop = global_event_loop()
> +   func = HangForever()
> +   

Re: [gentoo-portage-dev] [PATCH 1/4] Add ForkExecutor (bug 649588)

[Alec Warner]

lgtm.

On Sat, Mar 31, 2018 at 10:46 PM, Zac Medico  wrote:

> This is useful for asynchronous operations that we might
> need to cancel if they take too long, since (concurrent.
> futures.ProcessPoolExecutor tasks are not cancellable).
> This ability to cancel tasks makes this executor useful
> as an alternative to portage.exception.AlarmSignal.
>
> Also add an asyncio-compatible EventLoop.run_in_executor
> method the uses ForkExecutor as the default executor,
> which will later be used to implement the corresponding
> asyncio.AbstractEventLoop run_in_executor method.
>
> Bug: https://bugs.gentoo.org/649588
> ---
>  pym/portage/util/_eventloop/EventLoop.py  |  45 -
>  pym/portage/util/futures/executor/__init__.py |   0
>  pym/portage/util/futures/executor/fork.py | 130
> ++
>  3 files changed, 174 insertions(+), 1 deletion(-)
>  create mode 100644 pym/portage/util/futures/executor/__init__.py
>  create mode 100644 pym/portage/util/futures/executor/fork.py
>
> diff --git a/pym/portage/util/_eventloop/EventLoop.py
> b/pym/portage/util/_eventloop/EventLoop.py
> index f472a3dae..1574a6837 100644
> --- a/pym/portage/util/_eventloop/EventLoop.py
> +++ b/pym/portage/util/_eventloop/EventLoop.py
> @@ -24,6 +24,7 @@ except ImportError:
>  import portage
>  portage.proxy.lazyimport.lazyimport(globals(),
> 'portage.util.futures.futures:_EventLoopFuture',
> +   'portage.util.futures.executor.fork:ForkExecutor',
>  )
>
>  from portage import OrderedDict
> @@ -122,6 +123,7 @@ class EventLoop(object):
> self._idle_callbacks = OrderedDict()
> self._timeout_handlers = {}
> self._timeout_interval = None
> +   self._default_executor = None
>
> self._poll_obj = None
> try:
> @@ -721,6 +723,46 @@ class EventLoop(object):
> return self._handle(self.timeout_add(
> delay * 1000, self._call_soon_callback(callback,
> args)), self)
>
> +   def run_in_executor(self, executor, func, *args):
> +   """
> +   Arrange for a func to be called in the specified executor.
> +
> +   The executor argument should be an Executor instance. The
> default
> +   executor is used if executor is None.
> +
> +   Use functools.partial to pass keywords to the *func*.
> +
> +   @param executor: executor
> +   @type executor: concurrent.futures.Executor or None
> +   @param func: a function to call
> +   @type func: callable
> +   @return: a Future
> +   @rtype: asyncio.Future (or compatible)
> +   """
> +   if executor is None:
> +   executor = self._default_executor
> +   if executor is None:
> +   executor = ForkExecutor(loop=self)
> +   self._default_executor = executor
> +   return executor.submit(func, *args)
> +
> +   def close(self):
> +   """Close the event loop.
> +
> +   This clears the queues and shuts down the executor,
> +   and waits for it to finish.
> +   """
> +   executor = self._default_executor
> +   if executor is not None:
> +   self._default_executor = None
> +   executor.shutdown(wait=True)
> +
> +   if self._poll_obj is not None:
> +   close = getattr(self._poll_obj, 'close')
> +   if close is not None:
> +   close()
> +   self._poll_obj = None
> +
>
>  _can_poll_device = None
>
> @@ -782,10 +824,11 @@ class _epoll_adapter(object):
> that is associated with an epoll instance will close automatically
> when
> it is garbage collected, so it's not necessary to close it
> explicitly.
> """
> -   __slots__ = ('_epoll_obj',)
> +   __slots__ = ('_epoll_obj', 'close')
>
> def __init__(self, epoll_obj):
> self._epoll_obj = epoll_obj
> +   self.close = epoll_obj.close
>
> def register(self, fd, *args):
> self._epoll_obj.register(fd, *args)
> diff --git a/pym/portage/util/futures/executor/__init__.py
> b/pym/portage/util/futures/executor/__init__.py
> new file mode 100644
> index 0..e69de29bb
> diff --git a/pym/portage/util/futures/executor/fork.py
> b/pym/portage/util/futures/executor/fork.py
> new file mode 100644
> index 0..9cd1db2ca
> --- /dev/null
> +++ b/pym/portage/util/futures/executor/fork.py
> @@ -0,0 +1,130 @@
> +# Copyright 2018 Gentoo Foundation
> +# Distributed under the terms of the GNU General Public License v2
> +
> +import collections
> +import functools
> +import multiprocessing
> +import os
> +import sys
> +import traceback
> +
> +from 

Re: [gentoo-portage-dev] [PATCH] INSTALL_MASK: honor install time config for binary packages (bug 651952)

[Michał Górny]

W dniu czw, 29.03.2018 o godzinie 15∶34 -0700, użytkownik Zac Medico
napisał:
> For binary packages, honor the INSTALL_MASK configuration that
> exists at install time, since it might differ from the build time
> setting.
> 
> Fixes: 3416876c0ee7 ("{,PKG_}INSTALL_MASK: python implementation")
> Bug: https://bugs.gentoo.org/651952
> ---
>  bin/misc-functions.sh| 23 +++
>  bin/phase-functions.sh   | 10 +-
>  pym/portage/dbapi/vartree.py |  5 +
>  3 files changed, 29 insertions(+), 9 deletions(-)
> 
> diff --git a/bin/misc-functions.sh b/bin/misc-functions.sh
> index 26f589915..a6330ee93 100755
> --- a/bin/misc-functions.sh
> +++ b/bin/misc-functions.sh
> @@ -323,6 +323,29 @@ postinst_qa_check() {
>   done < <(printf "%s\0" "${qa_checks[@]}" | LC_ALL=C sort -u -z)
>  }
>  
> +preinst_mask() {
> + # Remove man pages, info pages, docs if requested. This is
> + # implemented in bash in order to respect INSTALL_MASK settings
> + # from bashrc.
> + local f x
> + for f in man info doc; do
> + if has no${f} ${FEATURES}; then
> + INSTALL_MASK+=" /usr/share/${f}"
> + fi
> + done
> +
> + # Store modified variables in build-info.
> + cd "${PORTAGE_BUILDDIR}"/build-info || die
> + set -f
> +
> + IFS=$' \t\n\r'
> + for f in INSTALL_MASK; do

This loop along with the whole indirection is entirely pointless, given
that you're processing exactly one variable.

> + x=$(echo -n ${!f})
> + [[ -n ${x} ]] && echo "${x}" > "${f}"

There's probably no point in this [[ -n ... ]], as that:

a. requires you to special-handle missing INSTALL_MASK file, while it's
easier to just ensure that it's there (and I think you requested
the same thing from me before you rewritten my commit into breakage),

b. makes it impossible to distinguish packages from before INSTALL_MASK
storing was added from those where it is empty.

> + done
> + set +f
> +}
> +
>  preinst_sfperms() {
>   if [ -z "${D}" ]; then
>eerror "${FUNCNAME}: D is unset"
> diff --git a/bin/phase-functions.sh b/bin/phase-functions.sh
> index bdae68f79..3de8d01b5 100644
> --- a/bin/phase-functions.sh
> +++ b/bin/phase-functions.sh
> @@ -661,14 +661,6 @@ __dyn_install() {
>   set -f
>   local f x
>  
> - # remove man pages, info pages, docs if requested
> - for f in man info doc; do
> - if has no${f} ${FEATURES} && \
> - ! has "/usr/share/${f}" ${INSTALL_MASK}; then
> - INSTALL_MASK+=" /usr/share/${f}"
> - fi
> - done
> -
>   IFS=$' \t\n\r'
>   for f in CATEGORY DEFINED_PHASES FEATURES INHERITED IUSE \
>   PF PKGUSE SLOT KEYWORDS HOMEPAGE DESCRIPTION \
> @@ -676,7 +668,7 @@ __dyn_install() {
>   CXXFLAGS EXTRA_ECONF EXTRA_EINSTALL EXTRA_MAKE \
>   LDFLAGS LIBCFLAGS LIBCXXFLAGS QA_CONFIGURE_OPTIONS \
>   QA_DESKTOP_FILE QA_PREBUILT PROVIDES_EXCLUDE REQUIRES_EXCLUDE \
> - INSTALL_MASK PKG_INSTALL_MASK; do
> + PKG_INSTALL_MASK; do
>  
>   x=$(echo -n ${!f})
>   [[ -n $x ]] && echo "$x" > $f
> diff --git a/pym/portage/dbapi/vartree.py b/pym/portage/dbapi/vartree.py
> index 378d42dc0..a136c38f1 100644
> --- a/pym/portage/dbapi/vartree.py
> +++ b/pym/portage/dbapi/vartree.py
> @@ -3846,6 +3846,11 @@ class dblink(object):
>   # be useful to avoid collisions in some scenarios.
>   # We cannot detect if this is needed or not here as 
> INSTALL_MASK can be
>   # modified by bashrc files.
> + phase = MiscFunctionsProcess(background=False,
> + commands=["preinst_mask"], phase="preinst",
> + scheduler=self._scheduler, settings=self.settings)
> + phase.start()
> + phase.wait()
>   try:
>   with io.open(_unicode_encode(os.path.join(inforoot, 
> "INSTALL_MASK"),
>   encoding=_encodings['fs'], errors='strict'),

-- 
Best regards,
Michał Górny

Re: [gentoo-portage-dev] [PATCH 0/4] rsync: add key refresh retry (bug 649276)

[Michał Górny]

W dniu sob, 31.03.2018 o godzinie 19∶46 -0700, użytkownik Zac Medico
napisał:
> Since key refresh is prone to failure, retry using exponential
> backoff with random jitter. This adds the following sync-openpgp-*
> configuration settings:
> 
> sync-openpgp-key-refresh-retry-count = 40
> 
>   Maximum number of times to retry key refresh if it fails.  Between
>   each  key  refresh attempt, there is an exponential delay with a
>   constant multiplier and a uniform random multiplier between 0 and 1.
> 
> sync-openpgp-key-refresh-retry-delay-exp-base = 2
> 
>   The base of the exponential expression. The exponent is the number
>   of previous refresh attempts.
> 
> sync-openpgp-key-refresh-retry-delay-max = 60
> 
>   Maximum  delay between each retry attempt, in units of seconds. This
>   places a limit on the length of the exponential delay.
> 
> sync-openpgp-key-refresh-retry-delay-mult = 4
> 
>   Multiplier for the exponential delay.
> 
> sync-openpgp-key-refresh-retry-overall-timeout = 1200
> 
>   Combined time limit for all refresh attempts, in units of seconds.
> 
> Bug: https://bugs.gentoo.org/649276
> 
> Zac Medico (4):
>   Add ForkExecutor (bug 649588)
>   Add ExponentialBackoff and RandomExponentialBackoff
>   Add retry decorator (API inspired by tenacity)
>   rsync: add key refresh retry (bug 649276)
> 
>  cnf/repos.conf|   5 +
>  man/portage.5 |  19 +++
>  pym/portage/repository/config.py  |  22 
>  pym/portage/sync/modules/rsync/rsync.py   |  16 ++-
>  pym/portage/sync/syncbase.py  |  85 +++-
>  pym/portage/tests/util/futures/test_retry.py  | 147 +
>  pym/portage/util/_eventloop/EventLoop.py  |  45 ++-
>  pym/portage/util/backoff.py   |  48 +++
>  pym/portage/util/futures/executor/__init__.py |   0
>  pym/portage/util/futures/executor/fork.py | 130 +++
>  pym/portage/util/futures/futures.py   |   6 +
>  pym/portage/util/futures/retry.py | 178 
> ++
>  12 files changed, 697 insertions(+), 4 deletions(-)
>  create mode 100644 pym/portage/tests/util/futures/test_retry.py
>  create mode 100644 pym/portage/util/backoff.py
>  create mode 100644 pym/portage/util/futures/executor/__init__.py
>  create mode 100644 pym/portage/util/futures/executor/fork.py
>  create mode 100644 pym/portage/util/futures/retry.py
> 

This essentially looks like ~700 lines of code to try to workaround
broken networking. I would rather try to do that using 5 lines of code
but that's just me, and my programs aren't enterprise quality. I just
hope it actually solves as many problems as it's going to introduce.

-- 
Best regards,
Michał Górny