Re: [Trac] Re: failed attempts to use a custom EmailResolver

2017-01-26 Thread Sandro Santilli
On Wed, Jan 25, 2017 at 07:56:36PM +0100, Peter Suter wrote:
> On 18.01.2017 09:43, Sandro Santilli wrote:
> >
> >The RecipientMatcher class is not documented so it is not clear
> >what the expected input is.
>
> The input is some string found in a ticket field. It could be an email
> address, a username or just a plain name, or even something (anything) else.
> RecipientMatcher is trying to find matching known usernames or valid email
> addresses that could be used as recipients.

In our case we need the EmailResolver to tell what can or cannot be used
as a recipient, but RecipientMatcher does not use EmailResolver.
The *caller* does, and only does IF RecipientMatcher does not return None,
which currently does unless the "sid" matches an authenticated user.

> The reason that RecipientMatcher does exactly that is mainly backward
> compatibility. Those are the things that have been supported historically,
> so they should continue to work.

Do you think EmailResolver returning a name even if it's not authenticated
could break something ? The returned tuple does have an "authenticated"
field, so could be used by caller not willing to deal with non-authenticated
strings ...

   https://git.osgeo.org/gogs/strk/trac/commit/a72b9176f

> >>As a last (and not very satisfying) resort, you could also replace all the
> >>INotificationSubscribers with your own implementations that work like you
> >>want I guess. :/
> >This would be no different than applying the patch above, right ?
> It would only be different in that you don't have to patch the source.

Got it, but sounds overkill at this stage.

--strk;

-- 
You received this message because you are subscribed to the Google Groups "Trac 
Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to trac-users+unsubscr...@googlegroups.com.
To post to this group, send email to trac-users@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-users.
For more options, visit https://groups.google.com/d/optout.


Re: [Trac] Re: failed attempts to use a custom EmailResolver

2017-01-25 Thread Peter Suter
Sorry, I just noticed some of your mails ended up in my spam folder for 
some reason.


On 18.01.2017 09:43, Sandro Santilli wrote:

On Tue, Jan 17, 2017 at 07:34:34PM +0100, Peter Suter wrote:

I think the problem is that we can't really know if these are actually
usernames, and at the moment the only thing we can reasonably do is check
against the known usernames.

What else could they be ?
The RecipientMatcher class is not documented so it is not clear
what the expected input is.
The input is some string found in a ticket field. It could be an email 
address, a username or just a plain name, or even something (anything) else.
RecipientMatcher is trying to find matching known usernames or valid 
email addresses that could be used as recipients.
The reason that RecipientMatcher does exactly that is mainly backward 
compatibility. Those are the things that have been supported 
historically, so they should continue to work.

As a last (and not very satisfying) resort, you could also replace all the
INotificationSubscribers with your own implementations that work like you
want I guess. :/

This would be no different than applying the patch above, right ?

It would only be different in that you don't have to patch the source.

--
You received this message because you are subscribed to the Google Groups "Trac 
Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to trac-users+unsubscr...@googlegroups.com.
To post to this group, send email to trac-users@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-users.
For more options, visit https://groups.google.com/d/optout.


Re: [Trac] Re: failed attempts to use a custom EmailResolver

2017-01-18 Thread Sandro Santilli
On Tue, Jan 17, 2017 at 07:34:34PM +0100, Peter Suter wrote:

> >https://git.osgeo.org/gogs/strk/trac/commit/a72b9176f

> I think the problem is that we can't really know if these are actually
> usernames, and at the moment the only thing we can reasonably do is check
> against the known usernames.

What else could they be ?
The RecipientMatcher class is not documented so it is not clear
what the expected input is.

> As a possible workaround for you, could you use (maybe in a scheduled
> script?) "trac-admin session add sid" to make all the users known?

In each of the dozen instances.. not so nice...

> As a last (and not very satisfying) resort, you could also replace all the
> INotificationSubscribers with your own implementations that work like you
> want I guess. :/

This would be no different than applying the patch above, right ?

--strk;

-- 
You received this message because you are subscribed to the Google Groups "Trac 
Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to trac-users+unsubscr...@googlegroups.com.
To post to this group, send email to trac-users@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-users.
For more options, visit https://groups.google.com/d/optout.


Re: [Trac] Re: failed attempts to use a custom EmailResolver

2017-01-17 Thread Sandro Santilli
On Tue, Jan 17, 2017 at 10:48:16AM +0100, Sandro Santilli wrote:
> On Thu, Jan 12, 2017 at 01:41:15PM -0800, RjOllos wrote:
> > On Thursday, January 12, 2017 at 1:15:59 AM UTC-8, Sandro Santilli wrote:
> > 
> > > But the patch to RecipientMatcher is still needed for the resolver 
> > > to be called. I filed it here: 
> > > https://trac.edgewall.org/ticket/12658 
> > 
> > The TracLDAPEmailResolverPlugin has inconsistent indentation, some 2-space 
> > and some 4-space. 
> 
> Fixed, thanks :)
> But even if a patch landed in 1.2 branch to fix RecipientMatcher so
> to allow the plugin for being used, it still only works for people
> that logged in at least once onto trac, which is not what we
> want for our use case. The thing is we might want to be able to
> notify (ie: put in Cc by nickname) someone who never logged yet
> (but we know he can log, being known by the LDAP database also used
> for authorizing logins).
> 
> Is there any reason why the RecipientMatcher should be checking
> for "known" users and only attempt to resolve emails for them ?

This simple patch (also referenced in the ticket now) is what allows
LDAP email to be retrived even for users that hadn't logged in yet:
https://git.osgeo.org/gogs/strk/trac/commit/a72b9176f

I've added a note about that in the ticket too (12658) but that ticket
is closed now, so not sure how to proceed.

--strk;

-- 
You received this message because you are subscribed to the Google Groups "Trac 
Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to trac-users+unsubscr...@googlegroups.com.
To post to this group, send email to trac-users@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-users.
For more options, visit https://groups.google.com/d/optout.


Re: [Trac] Re: failed attempts to use a custom EmailResolver

2017-01-17 Thread Sandro Santilli
On Thu, Jan 12, 2017 at 01:41:15PM -0800, RjOllos wrote:
> On Thursday, January 12, 2017 at 1:15:59 AM UTC-8, Sandro Santilli wrote:
> 
> > But the patch to RecipientMatcher is still needed for the resolver 
> > to be called. I filed it here: 
> > https://trac.edgewall.org/ticket/12658 
> 
> The TracLDAPEmailResolverPlugin has inconsistent indentation, some 2-space 
> and some 4-space. 

Fixed, thanks :)
But even if a patch landed in 1.2 branch to fix RecipientMatcher so
to allow the plugin for being used, it still only works for people
that logged in at least once onto trac, which is not what we
want for our use case. The thing is we might want to be able to
notify (ie: put in Cc by nickname) someone who never logged yet
(but we know he can log, being known by the LDAP database also used
for authorizing logins).

Is there any reason why the RecipientMatcher should be checking
for "known" users and only attempt to resolve emails for them ?

--strk;

-- 
You received this message because you are subscribed to the Google Groups "Trac 
Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to trac-users+unsubscr...@googlegroups.com.
To post to this group, send email to trac-users@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-users.
For more options, visit https://groups.google.com/d/optout.


Re: [Trac] Re: failed attempts to use a custom EmailResolver

2017-01-12 Thread RjOllos
On Thursday, January 12, 2017 at 1:15:59 AM UTC-8, Sandro Santilli wrote:
>
> On Wed, Jan 11, 2017 at 10:44:38PM +0100, Peter Suter wrote: 
> > On 11.01.2017 22:32, Sandro Santilli wrote: 
> > >2017-01-11 22:26:17,280 Trac[web_ui] ERROR: Failure sending 
> notification on change to ticket #777: TypeError: get_address_for_session() 
> takes exactly 2 arguments (3 given) 
> > 
> > The LdapEmailAddressResolverexample was missing the self parameter: 
> > 
> https://trac.edgewall.org/wiki/TracDev/PluginDevelopment/ExtensionPoints/trac.notification.api.IEmailAddressResolver?action=diff=6
>  
>
> Yes, thanks for fixing that. 
> But the patch to RecipientMatcher is still needed for the resolver 
> to be called. I filed it here: 
> https://trac.edgewall.org/ticket/12658 
>
> --strk; 
>

The TracLDAPEmailResolverPlugin has inconsistent indentation, some 2-space 
and some 4-space. You could fix with:

$ pip install reindent 
$ cd TracLDAPEmailResolverPlugin
$ reindent -r -n .

- Ryan

-- 
You received this message because you are subscribed to the Google Groups "Trac 
Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to trac-users+unsubscr...@googlegroups.com.
To post to this group, send email to trac-users@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-users.
For more options, visit https://groups.google.com/d/optout.


Re: [Trac] Re: failed attempts to use a custom EmailResolver

2017-01-12 Thread Sandro Santilli
On Wed, Jan 11, 2017 at 10:44:38PM +0100, Peter Suter wrote:
> On 11.01.2017 22:32, Sandro Santilli wrote:
> >2017-01-11 22:26:17,280 Trac[web_ui] ERROR: Failure sending notification on 
> >change to ticket #777: TypeError: get_address_for_session() takes exactly 2 
> >arguments (3 given)
> 
> The LdapEmailAddressResolverexample was missing the self parameter:
> https://trac.edgewall.org/wiki/TracDev/PluginDevelopment/ExtensionPoints/trac.notification.api.IEmailAddressResolver?action=diff=6

Yes, thanks for fixing that.
But the patch to RecipientMatcher is still needed for the resolver
to be called. I filed it here:
https://trac.edgewall.org/ticket/12658

--strk;

-- 
You received this message because you are subscribed to the Google Groups "Trac 
Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to trac-users+unsubscr...@googlegroups.com.
To post to this group, send email to trac-users@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-users.
For more options, visit https://groups.google.com/d/optout.


Re: [Trac] Re: failed attempts to use a custom EmailResolver

2017-01-11 Thread Peter Suter

On 11.01.2017 22:32, Sandro Santilli wrote:

2017-01-11 22:26:17,280 Trac[web_ui] ERROR: Failure sending notification on 
change to ticket #777: TypeError: get_address_for_session() takes exactly 2 
arguments (3 given)


The LdapEmailAddressResolverexample was missing the self parameter:
https://trac.edgewall.org/wiki/TracDev/PluginDevelopment/ExtensionPoints/trac.notification.api.IEmailAddressResolver?action=diff=6

--
You received this message because you are subscribed to the Google Groups "Trac 
Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to trac-users+unsubscr...@googlegroups.com.
To post to this group, send email to trac-users@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-users.
For more options, visit https://groups.google.com/d/optout.


Re: [Trac] Re: failed attempts to use a custom EmailResolver

2017-01-11 Thread Sandro Santilli
On Wed, Jan 11, 2017 at 10:32:25PM +0100, Sandro Santilli wrote:
> On Wed, Jan 11, 2017 at 10:47:41AM -0800, Peter Suter wrote:
> 
> > It might be a bug (or a scenario not supported so far).
> > Could you try replacing that last "return None" with "return address, 1, 
> > None"?
> 
> I got to the same point already, which gave me:
> 
> 2017-01-11 22:26:17,280 Trac[web_ui] ERROR: Failure sending notification on 
> change to ticket #777: TypeError: get_address_for_session() takes exactly 2 
> arguments (3 given)
> 
> Hadn't figured out why it's said to take 2 args as I only see
> a  3-args one (first of which being "self"). Unfortunately there's no
> stack trace in that error.

Bingo, it's actually working (was my plugin being bogus, as the Example
I was following [1]). Should I send a patch for it ?

[1] 
https://trac.edgewall.org/wiki/TracDev/PluginDevelopment/ExtensionPoints/trac.notification.api.IEmailAddressResolver#Examples

So the change in mail.py works great:

  233c235,236
  < return None
  ---
  > #return None
  > return (address, 1, None)

Only maybe we should also drop the complaint:

  INFO: Email address w/o domain: xxx

As that'd be a legit kind of "recipient" when a resolver is in use

--strk;

-- 
You received this message because you are subscribed to the Google Groups "Trac 
Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to trac-users+unsubscr...@googlegroups.com.
To post to this group, send email to trac-users@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-users.
For more options, visit https://groups.google.com/d/optout.


Re: [Trac] Re: failed attempts to use a custom EmailResolver

2017-01-11 Thread Sandro Santilli
On Wed, Jan 11, 2017 at 10:47:41AM -0800, Peter Suter wrote:

> It might be a bug (or a scenario not supported so far).
> Could you try replacing that last "return None" with "return address, 1, 
> None"?

I got to the same point already, which gave me:

2017-01-11 22:26:17,280 Trac[web_ui] ERROR: Failure sending notification on 
change to ticket #777: TypeError: get_address_for_session() takes exactly 2 
arguments (3 given)

Hadn't figured out why it's said to take 2 args as I only see
a  3-args one (first of which being "self"). Unfortunately there's no
stack trace in that error.

> Announcer uses "(sid, 1, None)" when an email address is not recognized:
> https://trac-hacks.org/browser/announcerplugin/trunk/announcer/subscribers.py#L151
> 
> We should maybe use "(sid, 1, None)" if RecipientMatcher returns None.

There's no documentation about what RecipientMatcher is supposed to do
so I cannot comment about this.

--strk;

-- 
You received this message because you are subscribed to the Google Groups "Trac 
Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to trac-users+unsubscr...@googlegroups.com.
To post to this group, send email to trac-users@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-users.
For more options, visit https://groups.google.com/d/optout.


[Trac] Re: failed attempts to use a custom EmailResolver

2017-01-11 Thread Peter Suter
On Wednesday, January 11, 2017 at 4:42:56 PM UTC+1, Sandro Santilli wrote:
>
> I've been trying all day to make the LdalEmailAddressResolver 
> work for my setup but failed miserably. 
>
> The example: 
>
> https://trac.edgewall.org/wiki/TracDev/PluginDevelopment/ExtensionPoints/trac.notification.api.IEmailAddressResolver#Examples
>  
>
> My setup is Apache taking care of the authentication so that trac 
> only has usernames to work with, and no emails at all. 
>
> Adding debugging code brought me to understand that the "distribute" 
> method of the INotificationDistributor implementation is not even 
> entered unless the RecipientMatcher provides it with an address, 
> which is not the case when the domain is unknown. Code block I'm 
> looking at is notification/mail.py:235: 
>
> elif not is_email(address) and self.nodomaddr_re.match(address): 
> if self.env.config.getbool('notification', 'use_short_addr'): 
> return (None, 0, address) 
> domain = self.env.config.get('notification', 
>  'smtp_default_domain') 
> if domain: 
> address = "%s@%s" % (address, domain) 
> else: 
> self.env.log.info("Email address w/o domain: %s", 
> address) 
> return None 
>
> There we usually plug our custom code, looking up email from LDAP 
> instead o complaining "Email address w/out domain". I was hoping this 
> would be done automatically but there's clearly no code doing that 
> in that method. Is it a bug or a design choice for some reason ? 
>


It might be a bug (or a scenario not supported so far).
Could you try replacing that last "return None" with "return address, 1, 
None"?

Announcer uses "(sid, 1, None)" when an email address is not recognized:
https://trac-hacks.org/browser/announcerplugin/trunk/announcer/subscribers.py#L151

We should maybe use "(sid, 1, None)" if RecipientMatcher returns None.

 

>
> I've been logging some of my findings in the ticket but then was asked 
> to continue here: https://trac.edgewall.org/ticket/3162#comment:16 
>
> Background information about our customization can be found here: 
> https://trac.osgeo.org/osgeo/ticket/39 
>
> Thanks in advance for any info you may give me to help with dropping the 
> custom modification and finally be upgrade-happy again. 
>
> --strk; 
>


Thanks,
Peter 

-- 
You received this message because you are subscribed to the Google Groups "Trac 
Users" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to trac-users+unsubscr...@googlegroups.com.
To post to this group, send email to trac-users@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-users.
For more options, visit https://groups.google.com/d/optout.