[opensuse-buildservice] [PATCH] Notifications do not support multiple action requests
Hello, I was trying out multiple actions in one request and noticed that the notifications contain only the last action. I looked at the code and found this in /usr/lib/obs/server/BSNotify.pm line 82 : # FIXME: how to handle multiple actions in one request here ? # right now the last one just wins .... So here's a proposed fix : add an actions key in the reqinfo hash, that contains an ordered list of hashes that contain the details of the actions. The Data::Dumper of reqinfo produced by the attached patch looks like this : $VAR1 = { 'when' => '2010-09-30T00:12:13', 'actions' => [ { 'sourcerevision' => '1', 'sourceproject' => 'Foo:Devel:Personal', 'deletepackage' => undef, 'targetproject' => 'Foo:Trunk', 'sourcepackage' => 'libfoo', 'type' => 'submit', 'targetpackage' => 'libfoo', 'deleteproject' => undef }, { 'sourcerevision' => '1', 'sourceproject' => 'Foo:Devel:Personal', 'deletepackage' => undef, 'targetproject' => 'Foo:Trunk', 'sourcepackage' => 'bar', 'type' => 'submit', 'targetpackage' => 'bar', 'deleteproject' => undef } ], 'author' => 'bob', 'description' => 'Test', 'state' => 'new', 'comment' => undef, 'who' => 'bob', 'sender' => 'bob', 'id' => 32, 'type' => '' }; Is this change acceptable ? If so, I guess the notification plugins will have to be changed to accommodate this. Thanks, Islam Amer
Hi Islam, thanks a lot for your patch, Klaas is responsible for this code parts and he will review it next week (he is on vacation this week). so please stay some more days patient with an answer. thanks adrian Am Donnerstag, 30. September 2010, 02:33:14 schrieb Islam Amer:
Hello,
I was trying out multiple actions in one request and noticed that the notifications contain only the last action.
I looked at the code and found this in /usr/lib/obs/server/BSNotify.pm line 82 :
# FIXME: how to handle multiple actions in one request here ? # right now the last one just wins ....
So here's a proposed fix : add an actions key in the reqinfo hash, that contains an ordered list of hashes that contain the details of the actions.
The Data::Dumper of reqinfo produced by the attached patch looks like this :
$VAR1 = { 'when' => '2010-09-30T00:12:13', 'actions' => [ { 'sourcerevision' => '1', 'sourceproject' => 'Foo:Devel:Personal', 'deletepackage' => undef, 'targetproject' => 'Foo:Trunk', 'sourcepackage' => 'libfoo', 'type' => 'submit', 'targetpackage' => 'libfoo', 'deleteproject' => undef }, { 'sourcerevision' => '1', 'sourceproject' => 'Foo:Devel:Personal', 'deletepackage' => undef, 'targetproject' => 'Foo:Trunk', 'sourcepackage' => 'bar', 'type' => 'submit', 'targetpackage' => 'bar', 'deleteproject' => undef } ], 'author' => 'bob', 'description' => 'Test', 'state' => 'new', 'comment' => undef, 'who' => 'bob', 'sender' => 'bob', 'id' => 32, 'type' => '' };
Is this change acceptable ? If so, I guess the notification plugins will have to be changed to accommodate this.
Thanks, Islam Amer
-- Adrian Schroeter SUSE Linux Products GmbH email: adrian@suse.de -- To unsubscribe, e-mail: opensuse-buildservice+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-buildservice+help@opensuse.org
On Thursday 30 September 2010 10:01:28 Adrian Schröter wrote:
Hi Islam, thanks for working on that. That is a long outstanding feature request.
I was trying out multiple actions in one request and noticed that the notifications contain only the last action.
I looked at the code and found this in /usr/lib/obs/server/BSNotify.pm line 82 :
# FIXME: how to handle multiple actions in one request here ? # right now the last one just wins ....
So here's a proposed fix : add an actions key in the reqinfo hash, that contains an ordered list of hashes that contain the details of the actions. That is, in our case, only half the way. We're using Hermes as notification system. As your patch changes the format of what is sent to Hermes via http we have to make Hermes understanding that as well.
I think your proposal is great, but please remember that we need to change the notification systems before we can deploy it. Are you also using Hermes or another system? Thanks for your help, Klaas -- To unsubscribe, e-mail: opensuse-buildservice+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-buildservice+help@opensuse.org
Hello Klaas, Welcome back :) We are using a different notification plugin (AMQP, BOSS). I can fix the hermes notification plugin, but I don't know where to start with hermes itself. But at least is the basic idea of the patch acceptable ? Thanks, Islam Amer On 10/05/2010 03:56 PM, ext Klaas Freitag wrote:
On Thursday 30 September 2010 10:01:28 Adrian Schröter wrote:
Hi Islam,
thanks for working on that. That is a long outstanding feature request.
I was trying out multiple actions in one request and noticed that the notifications contain only the last action.
I looked at the code and found this in /usr/lib/obs/server/BSNotify.pm line 82 :
# FIXME: how to handle multiple actions in one request here ? # right now the last one just wins ....
So here's a proposed fix : add an actions key in the reqinfo hash, that contains an ordered list of hashes that contain the details of the actions. That is, in our case, only half the way. We're using Hermes as notification system. As your patch changes the format of what is sent to Hermes via http we have to make Hermes understanding that as well.
I think your proposal is great, but please remember that we need to change the notification systems before we can deploy it.
Are you also using Hermes or another system?
Thanks for your help,
Klaas -- To unsubscribe, e-mail: opensuse-buildservice+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-buildservice+help@opensuse.org
On Tuesday 05 October 2010 15:56:35 Islam Amer wrote: Hi,
Welcome back :)
Thanks :)
We are using a different notification plugin (AMQP, BOSS). I can fix the hermes notification plugin, but I don't know where to start with hermes itself.
But at least is the basic idea of the patch acceptable ?
Yes, absolutely. If that works with your notification plugin, we can agree on that change and I change Hermes accordingly. regards, Klaas
On 10/05/2010 03:56 PM, ext Klaas Freitag wrote:
On Thursday 30 September 2010 10:01:28 Adrian Schröter wrote:
Hi Islam,
thanks for working on that. That is a long outstanding feature request.
I was trying out multiple actions in one request and noticed that the notifications contain only the last action.
I looked at the code and found this in /usr/lib/obs/server/BSNotify.pm line 82 :
# FIXME: how to handle multiple actions in one request here ? # right now the last one just wins ....
So here's a proposed fix : add an actions key in the reqinfo hash, that contains an ordered list of hashes that contain the details of the actions. That is, in our case, only half the way. We're using Hermes as notification system. As your patch changes the format of what is sent to Hermes via http we have to make Hermes understanding that as well.
I think your proposal is great, but please remember that we need to change the notification systems before we can deploy it.
Are you also using Hermes or another system?
Thanks for your help,
Klaas
-- To unsubscribe, e-mail: opensuse-buildservice+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-buildservice+help@opensuse.org
Hello, Great then! I have already applied the patch to our OBS, and it is working fine. The AMQP and BOSS notification plugins didn't need to be modified because they just dump the data structure as is. Should I try to fix the hermes notification plugin as well ? I don't exactly know what hermes expects though :) Thanks, Islam Amer On 10/05/2010 05:19 PM, ext Klaas Freitag wrote:
On Tuesday 05 October 2010 15:56:35 Islam Amer wrote: Hi,
Welcome back :)
Thanks :)
We are using a different notification plugin (AMQP, BOSS). I can fix the hermes notification plugin, but I don't know where to start with hermes itself.
But at least is the basic idea of the patch acceptable ?
Yes, absolutely. If that works with your notification plugin, we can agree on that change and I change Hermes accordingly.
regards,
Klaas
On 10/05/2010 03:56 PM, ext Klaas Freitag wrote:
On Thursday 30 September 2010 10:01:28 Adrian Schröter wrote:
Hi Islam,
thanks for working on that. That is a long outstanding feature request.
I was trying out multiple actions in one request and noticed that the notifications contain only the last action.
I looked at the code and found this in /usr/lib/obs/server/BSNotify.pm line 82 :
# FIXME: how to handle multiple actions in one request here ? # right now the last one just wins ....
So here's a proposed fix : add an actions key in the reqinfo hash, that contains an ordered list of hashes that contain the details of the actions. That is, in our case, only half the way. We're using Hermes as notification system. As your patch changes the format of what is sent to Hermes via http we have to make Hermes understanding that as well.
I think your proposal is great, but please remember that we need to change the notification systems before we can deploy it.
Are you also using Hermes or another system?
Thanks for your help,
Klaas
-- To unsubscribe, e-mail: opensuse-buildservice+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-buildservice+help@opensuse.org
On Wednesday 06 October 2010 10:08:29 Islam Amer wrote:
Hello,
Great then! I have already applied the patch to our OBS, and it is working fine.
The AMQP and BOSS notification plugins didn't need to be modified because they just dump the data structure as is.
Should I try to fix the hermes notification plugin as well ? I don't exactly know what hermes expects though :) That would be great, because nowadays the Hermes plugin only works with one notification per request, because Hermes can not yet handle multiple notifications in one HTTP request. But that can and should be changed. If you change the plugin on backend side, I will fix Hermes for that.
Thanks, Klaas
On 10/05/2010 05:19 PM, ext Klaas Freitag wrote:
On Tuesday 05 October 2010 15:56:35 Islam Amer wrote: Hi,
Welcome back :)
Thanks :)
We are using a different notification plugin (AMQP, BOSS). I can fix the hermes notification plugin, but I don't know where to start with hermes itself.
But at least is the basic idea of the patch acceptable ?
Yes, absolutely. If that works with your notification plugin, we can agree on that change and I change Hermes accordingly.
regards,
Klaas
On 10/05/2010 03:56 PM, ext Klaas Freitag wrote:
On Thursday 30 September 2010 10:01:28 Adrian Schröter wrote:
Hi Islam,
thanks for working on that. That is a long outstanding feature request.
I was trying out multiple actions in one request and noticed that the notifications contain only the last action.
I looked at the code and found this in /usr/lib/obs/server/BSNotify.pm line 82 :
# FIXME: how to handle multiple actions in one request here ? # right now the last one just wins ....
So here's a proposed fix : add an actions key in the reqinfo hash, that contains an ordered list of hashes that contain the details of the actions. That is, in our case, only half the way. We're using Hermes as notification system. As your patch changes the format of what is sent to Hermes via http we have to make Hermes understanding that as well.
I think your proposal is great, but please remember that we need to change the notification systems before we can deploy it.
Are you also using Hermes or another system?
Thanks for your help,
Klaas
-- To unsubscribe, e-mail: opensuse-buildservice+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-buildservice+help@opensuse.org
Hello Klaas, Sorry for the long delay, this patch was almost forgotten. I have applied this patch to a build-service clone at gitorious. This is still without fixing hermes, which I can't seem to be able to fix (or I am not trying hard enough :) ) http://gitorious.org/~iamer/opensuse/iamers-build-service/commit/9193efdb32e... I'll try to work on the hermes patch, any help is appreciated. On 10/05/2010 04:56 PM, Islam Amer wrote:
Hello Klaas,
Welcome back :)
We are using a different notification plugin (AMQP, BOSS). I can fix the hermes notification plugin, but I don't know where to start with hermes itself.
But at least is the basic idea of the patch acceptable ?
Thanks, Islam Amer
On 10/05/2010 03:56 PM, ext Klaas Freitag wrote:
On Thursday 30 September 2010 10:01:28 Adrian Schröter wrote:
Hi Islam,
thanks for working on that. That is a long outstanding feature request.
I was trying out multiple actions in one request and noticed that the notifications contain only the last action.
I looked at the code and found this in /usr/lib/obs/server/BSNotify.pm line 82 :
# FIXME: how to handle multiple actions in one request here ? # right now the last one just wins ....
So here's a proposed fix : add an actions key in the reqinfo hash, that contains an ordered list of hashes that contain the details of the actions. That is, in our case, only half the way. We're using Hermes as notification system. As your patch changes the format of what is sent to Hermes via http we have to make Hermes understanding that as well.
I think your proposal is great, but please remember that we need to change the notification systems before we can deploy it.
Are you also using Hermes or another system?
Thanks for your help,
Klaas
-- Thanks, Islam Amer
participants (3)
-
Adrian Schröter -
Islam Amer -
Klaas Freitag