On Sun, Dec 25, 2022 at 04:51:14AM +0000, John Rouillard wrote:
> 
> In message <1671650263.38.0.386812066679.issue2551250@roundup.psfhosted.org>,
> Ralf Schlatterbeck writes:
> >I've pushed a fix that fixes the sorting issue should there ever
> >be two detectors for the same event with the same name and prio.
> 
> Should two entries with the same name/prio even be allowed?

I don't see any good reason to specifically check for this.

> Since list.sort() is stable, identical entries will preserve the
> original relative order of identical keys, but that's not documented
> anywhere that I can see. In fact we don't document that identically
> prioritied and named items can be added. Given that it used to cause a
> crash, I assume it's not being used.

Yes I think this is a minor issue. In most use-cases you don't even assign
an explicit prio to detectors.

> >But: In my tracker I do not have duplicate detector names.
> >In fact I'm currently unable to reproduce the problem.
> 
> I assume you're trying to reproduce without the patch.  Do you have
> any debugging statements in place that may perturb things?  Also what
> version of Python are you running?

This is 3.8 on the production machine. Note that this will not occur on
python2 because there comparison of functions is allowed. I currently do
not yet have debug code in place (I just wanted to get out a fix before
christmas :-)

Also note: I'm *newly* getting the same error message on a tracker that
is running on python3 for quite some time (also 3.8 afair). So is it
possible that recently something has been introduce that duplicates
detectors? On the other hand this tracker is quite low-volume... so it
may well be that we never hat multiple incoming emails in a single
mailgw run yet.

> I wonder if we should raise an exception when adding an element with
> the same name and priority as an existing element. Detecting this at
> the source should identify where the duplicates are coming from. This
> might expose another bug. I am concerned about the performance issue
> but we could add the key (priority, name) to a dict and just compare
> the key to determine membership.

or a set()

> 
> Ideally we would detect the issue at time of creation, but wrapping:
> 
>   self.list.sort()
> 
> in try/except to capture the exception and dump self.list might
> provide some clues. I have been looking for changes between python 2
> and python 3 that could trigger this but I got nothing.

I don't think there were changes when introducing py3, the comparison
(for sorting) of functions works fine in python2 but not in python3:

: ralf@bee:3; python3
>>> def f1 ():
...  pass
...
>>> def f2 ():
...  pass
...
>>> f1 < f2
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: '<' not supported between instances of 'function' and 'function'

: ralf@bee:4; python2
...
>>> f1 < f2
True

> >Is it possible that under certain circumstances we add detectors twice?
> 
> I suggested that as a possibility in my response to your initial
> report.  I have no idea how that would happen (intermittently) though.

> >Note that this occurred also in the mailgw interface, maybe when
> >processing more than one mail this can occur?
> 
> Maybe. I assume you are fetching email via imap from cron?

It might occur when processing more than one email in the mailgw if
there is a bug that repeatedly adds detectors for each mail that is
processed. That's my current hypothesis...

And, yes, I'm using imap.

> Might:
> 
>    https://github.com/bamthomas/aioimaplib/blob/master/aioimaplib/tests/imapserver.py
> 
> or
> 
>   https://github.com/antespi/docker-imap-devel
> 
> or some other imap mock server be useful for testing?

Yes, unless we can reproduce it with some other mailgw method, it might
not be related to imap.

If we don't write each other, I wish you 'einen guten Rutsch ins Neue
Jahr' (roughly translates as 'a good slide to the new year' :-)

Ralf
-- 
Dr. Ralf Schlatterbeck                  Tel:   +43/2243/26465-16
Open Source Consulting                  www:   www.runtux.com
Reichergasse 131, A-3411 Weidling       email: office@runtux.com