Re: [pbs-devel] [PATCH proxmox-backup 2/3] server/email_notifications: do not panic on template registration

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

On 18.03.21 10:57, Dominik Csapak wrote:
> On 3/17/21 20:38, Thomas Lamprecht wrote:
>> On 16.03.21 12:56, Dominik Csapak wrote:
>>> instead print an error and continue, the rendering functions will error
>>> out if one of the templates could not be registered
>>>
>>> if we `.unwrap()` here, it can lead to problems if the templates are
>>> not correct, i.e. we could panic while holding a lock, if something holds
>>> a mutex while this is called for the first time
>>
>> how can they error?
>> And any error (with or without this patch) would lead to emails notification not
>> working anymore, some may seem this as quite fatal error if they do not get notified
>> on erroneous jobs anymore? We may not be able to do much here, that's why above
>> question about what the error source can be.
> 
> they can error if they do not compile, e.g. they have syntax errors
> while we should catch that during developement/reviewing,
> if it does happen, it generates some weird behaviour
> (for example panicing while holding a mutex)

can't we just add a test for that instead, so that it is actually "compile checked"
when building a package?

Then such errors would be actually fixed before getting released, relying on
review/test tends to fail and let slip something trhough sooner or later.
> 
> with my patch, we still generate a warning, but aside from
> sending notification mails (where we still would warn in the log)
> the rest should work fine, there we can ofc also error out on
> notification errors so that the tasks get an error
> (but a well defined one instead of a panic)
> 
> also we may want to put the templates into files in the future
> so that users can adapt it and we can more easily change
> them (maybe localize them?)
> 

would need a sane reload mechanism then though, and in any way the one we ship
should be tested for basic validity on build.