You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bloodhound.apache.org by Andrej Golcov <an...@digiverse.si> on 2013/04/09 23:12:23 UTC
Transaction resource changing events
Hi everyone,
As a part of the investigation for ticket relation proposal [1], I
would like to discuss one issue with the community: how to make
changes in a new ticket relation mapping table consistently with
ticket modification - IMO, ticket relation changes should be a part of
the ticket modification transaction.
How the problem is solved by other implementations:
- MasterTicketPlugin [2] listens for ITicketChangeListener events and
updates mapping table according to specific custom ticket field - this
approach brings potential problems with DB consistency, because
ITicketChangeListener events are called after the ticket transaction
was committed.
- TicketLinks [3] branch modifies the Trac Ticket code to update
mapping table during the ticket db transaction - this approach is not
convenient for us, because we want to be close to Trac main branch as
much as possible.
My suggestion is to introduce a new interface
IResourceChangingListener that will be called before the main
transaction is committed and to put the ticket relation logics in a
Bloodhound specific plugin. I also hope that the interface could be
accepted by the Trac community, because it does not contain
ticket-relation specific code. Please consider the sample interface in
pastebin link: http://pastebin.com/BcH2AnS6
What do you think? Is it right direction to move?
[1] https://issues.apache.org/bloodhound/wiki/Proposals/BEP-0006
[2] http://trac-hacks.org/wiki/MasterTicketsPlugin
[3] http://trac.edgewall.org/wiki/TicketLinks
Cheers, Andrej
Re: Transaction resource changing events
Posted by Peter Koželj <pe...@digiverse.si>.
I am guessing that the new listener interface that would be called inside
of transaction
could also be used to ensure search indexing queue consistency.
The only concern that I have that the new interface would be misused to do
more work
inside transaction that would be strictly necessary. The documentation
needs to be very
clear here on what is allowed here.
On 9 April 2013 23:12, Andrej Golcov <an...@digiverse.si> wrote:
> Hi everyone,
>
> As a part of the investigation for ticket relation proposal [1], I
> would like to discuss one issue with the community: how to make
> changes in a new ticket relation mapping table consistently with
> ticket modification - IMO, ticket relation changes should be a part of
> the ticket modification transaction.
>
> How the problem is solved by other implementations:
> - MasterTicketPlugin [2] listens for ITicketChangeListener events and
> updates mapping table according to specific custom ticket field - this
> approach brings potential problems with DB consistency, because
> ITicketChangeListener events are called after the ticket transaction
> was committed.
> - TicketLinks [3] branch modifies the Trac Ticket code to update
> mapping table during the ticket db transaction - this approach is not
> convenient for us, because we want to be close to Trac main branch as
> much as possible.
>
> My suggestion is to introduce a new interface
> IResourceChangingListener that will be called before the main
> transaction is committed and to put the ticket relation logics in a
> Bloodhound specific plugin. I also hope that the interface could be
> accepted by the Trac community, because it does not contain
> ticket-relation specific code. Please consider the sample interface in
> pastebin link: http://pastebin.com/BcH2AnS6
>
> What do you think? Is it right direction to move?
>
> [1] https://issues.apache.org/bloodhound/wiki/Proposals/BEP-0006
> [2] http://trac-hacks.org/wiki/MasterTicketsPlugin
> [3] http://trac.edgewall.org/wiki/TicketLinks
>
> Cheers, Andrej
>
Re: Transaction resource changing events
Posted by Olemis Lang <ol...@gmail.com>.
On 4/10/13, Olemis Lang <ol...@gmail.com> wrote:
> On 4/10/13, Andrej Golcov <an...@digiverse.si> wrote:
>
[...]
>>> If you ask me I prefer (3) or (2) .
>> AFIK, "context" parameter is used in multiple places in Trac with
>> meaning of context e.g. in resource ResourceContext, in versioncontrol
>> in wiki formatters.
>
> That's the point . All those cases make reference to the rendering context
> .
>
Just take the time to make a quick research to realize that even if
IResourceManager.get_resource_description states that :type context:
`ResourceContext` , in real life life there's no such ResourceContext
class . That method accepts a rendering context i.e.
{{{
#!py
def get_resource_description(resource, format='default', context=None,
**kwargs):
"""Return a string representation of the resource, according to the
`format`.
:param resource: the `Resource` to describe
:param format: the kind of description wanted. Typical formats are:
`'default'`, `'compact'` or `'summary'`.
:param context: an optional rendering context to allow rendering rich
output (like markup containing links)
:type context: `ResourceContext`
[...]
"""
}}}
{{{
#!sh
$ grep -r ResourceContext trac
trac/resource.py: :type context: `ResourceContext`
}}}
that makes my point more clear . Variable name `context` is even used
in the same file for to completely unrelated purposes . That's
confusing .
[...]
--
Regards,
Olemis.
Re: Transaction resource changing events
Posted by Olemis Lang <ol...@gmail.com>.
On 4/10/13, Andrej Golcov <an...@digiverse.si> wrote:
>> ... a comment that's also valid for IResourceChangeListener . In Trac
>> , `context` is a variable name associated to instances of
>> `trac.mimeview.api.RenderingContext` (similar to `env` for
>> `trac.env.Environment`) . Therefore it's a bit confusing in this
>> context (at least for me that I'm familiar with that term and how it's
>> used in Trac code base ;) since it clashes with that inherited
>> *convention*. Is there a chance to name these objects somehow
>> different ?
>>
>> 1. evt (dojo)
>> 2. eventObject (jquery)
>> 3. EventArgs (C#)
>> 4. ...
>>
>> If you ask me I prefer (3) or (2) .
> AFIK, "context" parameter is used in multiple places in Trac with
> meaning of context e.g. in resource ResourceContext, in versioncontrol
> in wiki formatters.
That's the point . All those cases make reference to the rendering context .
> IMHO, by renaming the context parameter to
> event(Args|Object) we are losing meaning of this parameter - context
> in wich event is triggered.
It's the same thing ... all C# events are losing semantic information
because of using EventArgs rather than context ?
The fact is also that in Python context is also close to context
managers ... another unrelated concept .
> I will start discussion on Trac dev group
> on previously introduced IResourceChangeListener interface in order to
> get more feedback and see if it can accepted by Trac community.
>
Ok ... I'll add some notes in there to express the same opinions I've
been saying in here ... jftr
;)
>> Just a question . Are these methods supposed to replace manipulators ?
> No. Manipulators events are called before the modification transaction
> is created and can be used for the resource manipulation and
> validation. While IResourceChangingListener events are to be called
> after DB modification is done but before DB transaction is committed.
> These events can be used for additional DB actions that should be
> atomic with the resource DB modification. In our case, it is
> modification of the ticket-relation table atomically with the ticket
> CRUD operation.
>
ok understood .
:)
> I agree with Peter, that the usage of the interface should be very
> clearly documented in order to avoid possible abuse because long
> running transaction can influence the overall system performance.
>
that's what I was worried about . We have three iterations every time
a resource is modified . That will have an impact on performance .
>> Beyond that
>>
>> - add reparent events
>> - if `old_values` is considered to be yet another instance of
>> event arguments then all methods will have the same
>> interface
>> * which makes me wander whether they should be
>> merged into a single method plus event name
>> included as argument or another field in event args
>> object (i.e. `context` in signatures above)
>> * and , by doing so , subsystems would even be
>> able to define their own event types without
>> requiring major architectural refactoring .
>
> IMO, using granular interfaces provides documentation by code. While
> more generic events make developer to rely on documentation (that is
> not the strongest part of the open-source) and debugging.
>
I agree with you . But I'd always favor a flexibility architecture
over patching source code . In case a new event is added , or in case
there are particular events for a given resource then :
1. related components will not be able to
reuse the notifications mechanism when adding
their own resource related events
2. code will have to be modified in order to deal with it
--
Regards,
Olemis.
Re: Transaction resource changing events
Posted by Andrej Golcov <an...@digiverse.si>.
> ... a comment that's also valid for IResourceChangeListener . In Trac
> , `context` is a variable name associated to instances of
> `trac.mimeview.api.RenderingContext` (similar to `env` for
> `trac.env.Environment`) . Therefore it's a bit confusing in this
> context (at least for me that I'm familiar with that term and how it's
> used in Trac code base ;) since it clashes with that inherited
> *convention*. Is there a chance to name these objects somehow
> different ?
>
> 1. evt (dojo)
> 2. eventObject (jquery)
> 3. EventArgs (C#)
> 4. ...
>
> If you ask me I prefer (3) or (2) .
AFIK, "context" parameter is used in multiple places in Trac with
meaning of context e.g. in resource ResourceContext, in versioncontrol
in wiki formatters. IMHO, by renaming the context parameter to
event(Args|Object) we are losing meaning of this parameter - context
in wich event is triggered. I will start discussion on Trac dev group
on previously introduced IResourceChangeListener interface in order to
get more feedback and see if it can accepted by Trac community.
> Just a question . Are these methods supposed to replace manipulators ?
No. Manipulators events are called before the modification transaction
is created and can be used for the resource manipulation and
validation. While IResourceChangingListener events are to be called
after DB modification is done but before DB transaction is committed.
These events can be used for additional DB actions that should be
atomic with the resource DB modification. In our case, it is
modification of the ticket-relation table atomically with the ticket
CRUD operation.
I agree with Peter, that the usage of the interface should be very
clearly documented in order to avoid possible abuse because long
running transaction can influence the overall system performance.
> Beyond that
>
> - add reparent events
> - if `old_values` is considered to be yet another instance of
> event arguments then all methods will have the same
> interface
> * which makes me wander whether they should be
> merged into a single method plus event name
> included as argument or another field in event args
> object (i.e. `context` in signatures above)
> * and , by doing so , subsystems would even be
> able to define their own event types without
> requiring major architectural refactoring .
IMO, using granular interfaces provides documentation by code. While
more generic events make developer to rely on documentation (that is
not the strongest part of the open-source) and debugging.
Cheers, Andrej
Re: Transaction resource changing events
Posted by Olemis Lang <ol...@gmail.com>.
On 4/9/13, Gavin McDonald <ga...@16degrees.com.au> wrote:
>
[...]
>> The ideal solution would be to have a pastebin installed
> in
>> apache.org domain . Ryan and me have been tweaking
>> trachacks:PastebinPlugin user interface for that very same purpose in the
>> past . I'm hoping this will be available for us ...
>> eventually .
>>
>
> paste.apache.org
>
awesome ! thanks for the link
:)
--
Regards,
Olemis.
RE: Transaction resource changing events
Posted by Gavin McDonald <ga...@16degrees.com.au>.
> -----Original Message-----
> From: Olemis Lang [mailto:olemis@gmail.com]
> Sent: Wednesday, 10 April 2013 1:47 PM
> To: dev@bloodhound.apache.org
> Subject: Re: Transaction resource changing events
>
> On 4/9/13, Andrej Golcov <an...@digiverse.si> wrote:
> >> Would it not make more sense if this code were simply committed into
> >> an appropriate place in trunk? Or even better, quoted inline in this
> >> message, so that it gets archived along with the rest of the mailing
> >> list? AFAIU pastbin links are not eternal, which could make
> >> reconstructing this discussion from archives a bit of a chore.
>
> Pastebin.com links may be eternal , that depends on how they are created .
> For instance , all those I've created in the past for test reports are
public and
> <everlasting> . The ideal solution would be to have a pastebin installed
in
> apache.org domain . Ryan and me have been tweaking
> trachacks:PastebinPlugin user interface for that very same purpose in the
> past . I'm hoping this will be available for us ...
> eventually .
>
paste.apache.org
Gav...
> > I haven't sent the code in mail since python markup can be messed by
> > email
>
> +
> besides , at least outside the ASF , it's not a good practice to submit
such
> texts to public MLs
>
> ... to the point ...
>
> [...]
> > 'context' is an action context, may contain author, comment etc.
> > Context
> > content depends on a resource type.
>
> ... a comment that's also valid for IResourceChangeListener . In Trac ,
> `context` is a variable name associated to instances of
> `trac.mimeview.api.RenderingContext` (similar to `env` for
> `trac.env.Environment`) . Therefore it's a bit confusing in this context
(at least
> for me that I'm familiar with that term and how it's used in Trac code
base ;)
> since it clashes with that inherited *convention*. Is there a chance to
name
> these objects somehow different ?
>
> 1. evt (dojo)
> 2. eventObject (jquery)
> 3. EventArgs (C#)
> 4. ...
>
> If you ask me I prefer (3) or (2) .
>
> >
> > def get_subscribed_resources():
> > """
> > Implementation should return iterator of resource types for
which
> > a listener has to be notified.
> >
> > None or empty list means all types of resources.
> > """
> >
> > def match_resource(resource):
> > """Return whether the listener wants to process the given
> > resource."""
> >
>
> + , these should work exactly the same as listeners will do , though
> we still have to find the BH-spot of this subject (see #485)
>
> > def resource_creating(db, resource, context):
> [...]
> >
> > def resource_changing(db, resource, old_values, context):
> [...]
> >
> > def resource_deleting(db, resource, context):
> [...]
> >
>
> Just a question . Are these methods supposed to replace manipulators ?
>
> - If so , how will they veto subsequent processing ?
> - Else , isn't it a fact that methods listed above and
> manipulators have a lot of overlapping (duplicated
> functionality) ?
> * Or maybe the idea is to replace manipulators with
> this interface and provide backwards compatibility , just
> like has already been done with IResourceChangeListener ?
>
> Beyond that
>
> - add reparent events
> - if `old_values` is considered to be yet another instance of
> event arguments then all methods will have the same
> interface
> * which makes me wander whether they should be
> merged into a single method plus event name
> included as argument or another field in event args
> object (i.e. `context` in signatures above)
> * and , by doing so , subsystems would even be
> able to define their own event types without
> requiring major architectural refactoring .
>
> --
> Regards,
>
> Olemis.
Re: Transaction resource changing events
Posted by Olemis Lang <ol...@gmail.com>.
On 4/9/13, Andrej Golcov <an...@digiverse.si> wrote:
>> Would it not make more sense if this code were simply committed into an
>> appropriate place in trunk? Or even better, quoted inline in this
>> message, so that it gets archived along with the rest of the mailing
>> list? AFAIU pastbin links are not eternal, which could make
>> reconstructing this discussion from archives a bit of a chore.
Pastebin.com links may be eternal , that depends on how they are
created . For instance , all those I've created in the past for test
reports are public and «everlasting» . The ideal solution would be to
have a pastebin installed in apache.org domain . Ryan and me have been
tweaking trachacks:PastebinPlugin user interface for that very same
purpose in the past . I'm hoping this will be available for us ...
eventually .
> I haven't sent the code in mail since python markup can be messed by
> email
+
besides , at least outside the ASF , it's not a good practice to
submit such texts to public MLs
... to the point ...
[...]
> 'context' is an action context, may contain author, comment etc.
> Context
> content depends on a resource type.
... a comment that's also valid for IResourceChangeListener . In Trac
, `context` is a variable name associated to instances of
`trac.mimeview.api.RenderingContext` (similar to `env` for
`trac.env.Environment`) . Therefore it's a bit confusing in this
context (at least for me that I'm familiar with that term and how it's
used in Trac code base ;) since it clashes with that inherited
*convention*. Is there a chance to name these objects somehow
different ?
1. evt (dojo)
2. eventObject (jquery)
3. EventArgs (C#)
4. ...
If you ask me I prefer (3) or (2) .
>
> def get_subscribed_resources():
> """
> Implementation should return iterator of resource types for which
> a listener has to be notified.
>
> None or empty list means all types of resources.
> """
>
> def match_resource(resource):
> """Return whether the listener wants to process the given
> resource."""
>
+ , these should work exactly the same as listeners will do , though
we still have to find the BH-spot of this subject (see #485)
> def resource_creating(db, resource, context):
[...]
>
> def resource_changing(db, resource, old_values, context):
[...]
>
> def resource_deleting(db, resource, context):
[...]
>
Just a question . Are these methods supposed to replace manipulators ?
- If so , how will they veto subsequent processing ?
- Else , isn't it a fact that methods listed above and
manipulators have a lot of overlapping (duplicated
functionality) ?
* Or maybe the idea is to replace manipulators with
this interface and provide backwards compatibility , just
like has already been done with IResourceChangeListener ?
Beyond that
- add reparent events
- if `old_values` is considered to be yet another instance of
event arguments then all methods will have the same
interface
* which makes me wander whether they should be
merged into a single method plus event name
included as argument or another field in event args
object (i.e. `context` in signatures above)
* and , by doing so , subsystems would even be
able to define their own event types without
requiring major architectural refactoring .
--
Regards,
Olemis.
Re: Transaction resource changing events
Posted by Andrej Golcov <an...@digiverse.si>.
> Would it not make more sense if this code were simply committed into an
> appropriate place in trunk? Or even better, quoted inline in this
> message, so that it gets archived along with the rest of the mailing
> list? AFAIU pastbin links are not eternal, which could make
> reconstructing this discussion from archives a bit of a chore.
I haven't sent the code in mail since python markup can be messed by
email but you are right, pastebin is an external dependency.
Here is the code previously from the link above:
------------------------
class IResourceChangingListener(Interface):
"""Extension point interface for components that require notification
when a resource is in process of creation, modification, or deletion.
Events are called when action is in progress and allow listener to perform
database manipulation within the same transaction as parent action.
Because of the involved transaction, listener activity should be short
running.
'db' is instance of TransactionContextManager used by a parent
activity
'resource' parameters is instance of the a resource e.g. ticket, milestone
etc.
'context' is an action context, may contain author, comment etc. Context
content depends on a resource type.
"""
def get_subscribed_resources():
"""
Implementation should return iterator of resource types for which
a listener has to be notified.
None or empty list means all types of resources.
"""
def match_resource(resource):
"""Return whether the listener wants to process the given resource."""
def resource_creating(db, resource, context):
"""
Called when a resource is in creating phase.
"""
def resource_changing(db, resource, old_values, context):
"""Called when a resource is in changing phase.
`old_values` is a dictionary containing the previous values of the
resource properties that changed. Properties are specific for resource
type.
"""
def resource_deleting(db, resource, context):
"""Called when a resource is in deleting phase."""
def resource_version_deleting(db, resource, context):
"""Called when a version of a resource in deleting phase."""
Re: Transaction resource changing events
Posted by Branko Čibej <br...@wandisco.com>.
On 09.04.2013 23:12, Andrej Golcov wrote:
> Please consider the sample interface in
> pastebin link: http://pastebin.com/BcH2AnS6
Would it not make more sense if this code were simply committed into an
appropriate place in trunk? Or even better, quoted inline in this
message, so that it gets archived along with the rest of the mailing
list? AFAIU pastbin links are not eternal, which could make
reconstructing this discussion from archives a bit of a chore.
-- Brane
--
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com