Re: [pve-devel] [PATCH manager 2/2] Close #1295: Make apt notifications configurable

["Dominic Jäger" <d.jaeger@proxmox.com>]

On Wed, Apr 07, 2021 at 10:51:43AM +0200, Thomas Lamprecht wrote:
> On 07.04.21 10:30, Dominic Jäger wrote:
> > -# We assume that users with subscriptions want informations
> > -# about new packages.
> > -my $notify = ($info && $info->{status} eq 'Active') ? 1 : 0;
> > +my $notify = $dccfg->{notify_updates} // 1;
> 
> We may want to keep the default value the same, i.e.:
> 
> my $notify = $dccfg->{notify_updates} // ($info && $info->{status} eq 'Active');

Is there a reason why we assume that users without subscription do not want
such notifications?

As far as I see it, if we change it to
> $dccfg->{notify_updates} // 1
Then (until they change something)
- users with active subscription should _continue_ to get notifications
- enterprise repo configured but invalid subscription will continue to _not_
  get mails (because pveupdate exits with error 100)

Then the only change is that users
- without/invalid subscription and
- with only no-subscription-repo configured
will now suddenly get mails, but this is actually good?

We could also append a line "You can deactivate these notifications in the
Datacenter options" to the mail.

> 
> (the following is actually meant for the pve-cluster patch):
> I'd really prefer using a colon for new config property entries, and I can imagine
> that there will be more such switches in the future, so maybe start out with a format
> sting (like migration is there) and have something like:
> 
> 'notify: package-updates=1'
> 
> what do you think?
Done :) So the mentioned
> $dccfg->{notify_updates} // 1
is actually
> my $notify = $dccfg->{notify}->{package_updates} // 1;
already.