Access request object in ITicketChangeListener methods

[Andrej Golcov]

Hi all,

I'm implementing code listening on ITicketChangeListener interface calls. 

I would like to call add_warning in addition to logging just in case if something go wrong inside the method. The add_warning method requires request object that is not provided by ITicketChangeListener.

Is there another way to obtain request object other than get it as parameter?

Thanks in advance.

Regards, Andrej

 

[Christopher Nelson]

> I'm implementing code listening on ITicketChangeListener interface calls.
> I would like to call add_warning in addition to logging just in case if
> something go wrong inside the method. The add_warning method requires
> request object that is not provided by ITicketChangeListener.
>
> Is there another way to obtain request object other than get it as
> parameter?

I don't know but there may be a problem that the ticket change
listener may not be called within the context of a web page. Any code
which calls Ticket.save_changes() (?) will ultimately invoke
ITicketChangeListener methods. Perhaps add_warning() can handle a
null request.

[Ethan Jucovy]

On Mon, Dec 17, 2012 at 6:25 AM, Andrej Golcov <and...@digiverse.si> wrote:

I'm implementing code listening on ITicketChangeListener interface calls. 

I would like to call add_warning in addition to logging just in case if something go wrong inside the method. The add_warning method requires request object that is not provided by ITicketChangeListener.

I recently figured out a way to get a request object in ITicketChangeListener methods, by looking at each frame of the call stack until I find one, and exiting out of my code if I never find one:

  https://github.com/boldprogressives/trac-WorkflowNotificationPlugin/blob/master/workflow_notification/__init__.py#L137

But, as Chris says, you probably shouldn't do this, unless you have no other options.  If you can figure out some way to hang your code off an IRequestFilter instead, it'd probably be safer.

-Ethan

[Olemis Lang]

What about using ITicketManipulator ?
;)

--
Regards,

Olemis.

Blog ES: http://simelo-es.blogspot.com/
Blog EN: http://simelo-en.blogspot.com/

Featured article:

[Ethan Jucovy]

On Mon, Dec 17, 2012 at 3:02 PM, Olemis Lang <ole...@gmail.com> wrote:

> If you can figure out some way to hang your code off an
> IRequestFilter instead, it'd probably be safer.
>

What about using ITicketManipulator ?
;)

Even better!  I'd forgotten about that one, thanks.

-Ethan

[Steffen Hoffmann]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

The interface is not yet described like Carsten and Peter did for a
number of other ExtensionPointInterfaces [1], but there's always the
source for reference [2]. As the description tells, it's an
"Extension point interface for components that require notification
when tickets are created, modified, or deleted."

Components, no word about sessions, and if you compare it to other
interface definitions you'll see, that the missing request is on
purpose. This interface was not meant to work in an interaction context,
but to enforce side-effects of ticket changes on other Components.
That's it.

You may succeed to find an active session and attach i.e. a warning
message to it, but that you can doesn't mean you should do so. In fact,
you shouldn't. Chances are, that there are 10 or more concurrent active
sessions at that time. Which one is right? Decide on req.path_info or
similar? No, that's certainly asking for trouble.

As someone else already pointed out, there are ITemplateStreamFilter and
IRequestHandler. These interfaces allow plugins provide actions on
selected request in interactive session context, and you should really
find a way with them.

Alternatively you could trigger a notification message in Trac's
notification system, or - outside of Trac core - use the event driven
AnnouncerPlugin [5], that will provide a more flexible framework
including transports other than email (think: IM).

If you can't go one of these ways, it's likely your concept, that is
flawed, not an API deficiency, and you should start by discussing here,
what problem you want to solve in the first place.

Steffen Hoffmann


[1] http://trac.edgewall.org/wiki/TracDev/PluginDevelopment/ExtensionPoints
[2]
http://trac.edgewall.org/browser/trunk/trac/ticket/api.py#/ITicketChangeListener
[3]
http://trac.edgewall.org/wiki/TracDev/PluginDevelopment/ExtensionPoints/trac.web.api.IRequestFilter
[4]
http://trac.edgewall.org/wiki/TracDev/PluginDevelopment/ExtensionPoints/trac.web.api.IRequestHandler
[5] http://trac-hacks.org/wiki/AnnouncerPlugin
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlDPhOsACgkQ31DJeiZFuHce6ACZAfCanUpQ/osSo6awpoJygRjk
TEcAnRWaurzk55ilkgAEtlVYtqsdcYXc
=wJW2
-----END PGP SIGNATURE-----

[Steffen Hoffmann]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Agree, that you should rather act before and block undesired ticket
changes from happening in the first place, if that has been the desire
behind the original question.

Steffen Hoffmann

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlDPlVwACgkQ31DJeiZFuHf9jgCgy8plGv6ocs3inDNS189eKHb7
cmEAnjlkKxrUGbqxuTUFi/vyLpGXlfHL
=GGve
-----END PGP SIGNATURE-----

[Andrej Golcov]

Thanks everybody for the useful response.

May be I have to share more details regarding my question. 

In scope of Bloodhound project we are preparing prototype implementation of full text search (you can find more info on [1]) with respect of Trac Advanced Search proposal [2], search refactoring [3] and AdvancedSearchPlugin[4].  Currently, whoosh framework is used as a free-text-search backend.

The idea is indexing a resource (e.g. ticket) in a free-text-search backend (Whoosh or Lucene/Solr) after it was added/changed into DB. ITicketChangeListener looks like the most appropriate existing interface for this purpose. I don't think that  ITicketManipulator, IRequestFilter or IRequestHandler are appropriate. I wanted to use a request object to call add_warning in case of possible indexing errors, of cause, if the call is in web context.

Based on your feedback, I will rethink the design to use something similar to AnnouncerPlugin for errors notification.

BTW, ITicketChangeListener does not provide all events required to support external index consistency. But I would open another thread on this subject.

> As the description tells, it's an 
>  "Extension point interface for components that require notification 
>   when tickets are created, modified, or deleted." 

>
> Components, no word about sessions, and if you compare it to other 
> interface definitions you'll see, that the missing request is on 
> purpose. This interface was not meant to work in an interaction context, 
> but to enforce side-effects of ticket changes on other Components. 
> That's it. 

I would not  agree that component that subscribed on ticket changed event doesn't need to know in what context the call is happen (context can be a web request, trac-admin call or whatever). I believe there are a lot of cases for this, like WorkflowNotificationPlugin mentioned by Ethan. I agree that context should not be passed as parameter but, IMHO, some ortogonal context facility will be useful, may be something like "env.current_context()" function. 

[1] https://issues.apache.org/bloodhound/wiki/Proposals/BEP-0004

[2] http://trac.edgewall.org/wiki/AdvancedSearch

[3] http://trac.edgewall.org/wiki/SearchRefactoring

[4 http://trac-hacks.org/wiki/TracAdvancedSearchPlugin

Regards, Andrej

[Olemis Lang]

On 12/19/12, Andrej Golcov <and...@digiverse.si> wrote:
> Thanks everybody for the useful response.
>

Hi Andrej !

>
> IRequestHandler are appropriate. I wanted to use a request object to
> call add_warning in case of possible indexing errors, of cause, if the call
>
> is in web context.
>

you have a few choices :

1. like I mentioned the natural ITicketManipulator , but that does not seem
to work for you
2. force ITicketManipulator at some point ... if there's
a web request in context
3. bhdashboard.util.dummy_request
4. make the component implement both request aware interfaces
(e.g. IRequestFilter) and ITicketChangeListener , then store
request object in thread local storage (e.g. threading.local) .
I don't know if there's in Trac something like a decorator or
alike for these use cases .
5. however , it seems to me that you are still looking for hooks
operating at model level rather than ticket form validation
to ensure ubiquitous updates for index consistency . This
makes me wonder whether you are better served with
self.env.log.warning . This is related to my note below
about the reason for ITicketChangeListener being
context agnostic . I mean why would you call req.add_warning
while executing e.g. an admin command , an RPC request , ... ?

> Based on your feedback, I will rethink the design to use something similar
> to AnnouncerPlugin for errors notification.
>
> BTW, ITicketChangeListener does not provide all events required to support
> external index consistency. But I would open another thread on this
> subject.
>

it seems so , yes .
:(

>> As the description tells, it's an
>> "Extension point interface for components that require notification
>> when tickets are created, modified, or deleted."
>>
>> Components, no word about sessions, and if you compare it to other
>> interface definitions you'll see, that the missing request is on
>> purpose. This interface was not meant to work in an interaction context,
>> but to enforce side-effects of ticket changes on other Components.
>> That's it.
>
> I would not agree that component that subscribed on ticket changed event
> doesn't need to know in what context the call is happen (context can be a
> web request, trac-admin call or whatever). I believe there are a lot of
> cases for this, like WorkflowNotificationPlugin mentioned by Ethan. I agree
>

That's exactly why ITicketChangeListener does not make references to
web requests . It's operating at model level , therefore agnostic to
context .

> that context should not be passed as parameter but, IMHO, some ortogonal
> context facility will be useful, may be something like
> "env.current_context()" function.
>

hmmm ... Trac is a multi-thread / process web app . Something like
current context does not belong in environment instance but separate
clasess e.g. Request, RenderingContext ...