Re: fedmsg structure change?

2017-01-25 Thread Pierre-Yves Chibon 
On Wed, Jan 25, 2017 at 09:48:12AM +0100, Michal Novotny wrote:
>On Wed, Jan 25, 2017 at 9:21 AM, Pierre-Yves Chibon 
>wrote:
>  Well, in the first email on this thread there is an error from
>  fedmsg_meta so
>  there is likely one part of fedmsg_meta that won't like the missing
>  fields.
> 
>I proposed this PR in fmn.consumer to fix it:
>
> https://github.com/fedora-infra/fmn.consumer/pull/97/commits/fdfcec49d0197d345d53e441b2fb106473ec3dda

Thanks :)

Pierre
___
copr-devel mailing list -- copr-devel@lists.fedorahosted.org
To unsubscribe send an email to copr-devel-le...@lists.fedorahosted.org

Re: fedmsg structure change?

2017-01-25 Thread Pierre-Yves Chibon 
On Wed, Jan 25, 2017 at 09:05:03AM +0100, Michal Novotny wrote:
>On Tue, Jan 24, 2017 at 10:40 PM, Pierre-Yves Chibon 
>wrote:
> 
>  On Tue, Jan 24, 2017 at 01:36:59PM +0100, Michal Novotny wrote:
>  >    On Tue, Jan 24, 2017 at 11:44 AM, Pierre-Yves Chibon
>  
>  >    wrote:
>  >
>  >      On Tue, Jan 24, 2017 at 11:30:33AM +0100, Michal Novotny
>  wrote:
>  >      >Ã*  Ã*  Yes, untested changes got into production. We are
>  sorry. I am
>  >      currently
>  >      >Ã*  Ã*  working on a fix.
>  >
>  >      If that made it up to production, then we'll need to adjust
>  fedmsg_meta
>  >      to
>  >      support these messages as well.
>  >
>  >      fedmsg_meta should work for all our published messages
>  (published and
>  >      stored on
>  >      datagrepper).
>  >
>  >    Okay, these messages have all empty body (the "msg" attribute),
>  e.g
>  >    `build.end`:
>  >
>  >  {
>  >    "source_name": "datanommer",
>  >    "i": 3,
>  ...
>  >    "timestamp": 1485166967.0,
>  >    "msg_id": "2017-db23086c-2001-496e-a59f-10b92c466ae5",
>  >    "topic": "org.fedoraproject.prod.copr.build.end",
>  >    "source_version": "0.6.5",
>  >    "signature":
>  
> "HJEOhP93vZFUk3cjIzggxZqIT+IRaLpKF/t21Kn0AcQ9B1VJEe+myerAAJMZfuXppGQqsFyzcPFx\nu+9p7geI5NqNxnN+diUXNlxbXN9/VN0X3vX7U4mbc0/zLyGcbKWIn/pcskbM5qYC2lJHLov0pMwq\nrbq/B0N3CxBL2og0Fj8=\n",
>  >    "msg": {}
>  >  }
>  >
>  >    Does fedmsg_meta need to be adjusted even so?
> 
>  Yeah, fedmsg_meta should be able to process all the messages stored in
>  datagrepper, otherwise it breaks datagrepper and a few other apps.
> 
>  So we'll end up with some not so useful message: "a build ended in copr"
>  but we
>  should add them.
> 
>A malformed message (processed with the current fedmsg_meta:
> 
>copr.build.start  Automated build of the None copr has been started JSON
> 
>fedmsg_meta might already account for the case of missing fields. Looking
>at the
> 
>code, I think this is the case.  Let me, please, know what you think.

Well, in the first email on this thread there is an error from fedmsg_meta so
there is likely one part of fedmsg_meta that won't like the missing fields.


Pierre
___
copr-devel mailing list -- copr-devel@lists.fedorahosted.org
To unsubscribe send an email to copr-devel-le...@lists.fedorahosted.org

Re: fedmsg structure change?

2017-01-25 Thread Michal Novotny 
On Tue, Jan 24, 2017 at 10:40 PM, Pierre-Yves Chibon 
wrote:

> On Tue, Jan 24, 2017 at 01:36:59PM +0100, Michal Novotny wrote:
> >On Tue, Jan 24, 2017 at 11:44 AM, Pierre-Yves Chibon <
> pin...@pingoured.fr>
> >wrote:
> >
> >  On Tue, Jan 24, 2017 at 11:30:33AM +0100, Michal Novotny wrote:
> >  >Â  Â  Yes, untested changes got into production. We are sorry. I am
> >  currently
> >  >Â  Â  working on a fix.
> >
> >  If that made it up to production, then we'll need to adjust
> fedmsg_meta
> >  to
> >  support these messages as well.
> >
> >  fedmsg_meta should work for all our published messages (published
> and
> >  stored on
> >  datagrepper).
> >
> >Okay, these messages have all empty body (the "msg" attribute), e.g
> >`build.end`:
> >
> >  {
> >"source_name": "datanommer",
> >"i": 3,
> ...
> >"timestamp": 1485166967.0,
> >"msg_id": "2017-db23086c-2001-496e-a59f-10b92c466ae5",
> >"topic": "org.fedoraproject.prod.copr.build.end",
> >"source_version": "0.6.5",
> >"signature": "HJEOhP93vZFUk3cjIzggxZqIT+IRaLpKF/t21Kn0AcQ9B1VJEe+
> myerAAJMZfuXppGQqsFyzcPFx\nu+9p7geI5NqNxnN+diUXNlxbXN9/
> VN0X3vX7U4mbc0/zLyGcbKWIn/pcskbM5qYC2lJHLov0pMwq\nrbq/B0N3CxBL2og0Fj8=\n",
> >"msg": {}
> >  }
> >
> >Does fedmsg_meta need to be adjusted even so?
>
> Yeah, fedmsg_meta should be able to process all the messages stored in
> datagrepper, otherwise it breaks datagrepper and a few other apps.
>
> So we'll end up with some not so useful message: "a build ended in copr"
> but we
> should add them.
>

A malformed message (processed with the current fedmsg_meta:

*copr.build.start*

Automated
build of the None copr has been started JSON


fedmsg_meta might already account for the case of missing fields. Looking
at the

code, I think this is the case.  Let me, please, know what you think.

>
> The easiest for this type of changes is to use test-driven-development,
> add the
> new message to the test, fill in the expectations and then adjust the
> processor
> to handle those cases.
>
> Pierre
> ___
> copr-devel mailing list -- copr-devel@lists.fedorahosted.org
> To unsubscribe send an email to copr-devel-le...@lists.fedorahosted.org
>
___
copr-devel mailing list -- copr-devel@lists.fedorahosted.org
To unsubscribe send an email to copr-devel-le...@lists.fedorahosted.org

Re: fedmsg structure change?

2017-01-24 Thread Pierre-Yves Chibon 
On Tue, Jan 24, 2017 at 03:12:10PM +0100, Michal Novotny wrote:
>Otherwise, in the fix that I am going to apply tomorrow, there will some
>additional fields for chroot.start and build.start messages.
> 
>For chroot.start:
>- status (int)
>- version (str)
>For build.start:
>- status (int)
>- version (str)
>- chroot (str)
> 
>The old fields will be all there with the same type. Only some new fields
>will be added. I hope that does not represent a problem for fedmsg_meta or
>anybody. I am aware that an ideal way would be to just leave the old
>message interface as it was but the code would get very ugly.  Of course,
>I will do it if needed.

Adding new fields is not a problem for fedmsg_meta as long as it can access the
old ones.

We can adjust fedmsg_meta to benefit from the new fields if we want. The way is
then to rename the tests TestFoo to LegacyTestFoo (so we keep supporting the old
formats) and then add a new TestFoo where the message is of the new format and
adjust the processor to support the new format (while still supporting the old
one).

For example:
https://github.com/fedora-infra/fedmsg_meta_fedora_infrastructure/pull/403


Pierre
___
copr-devel mailing list -- copr-devel@lists.fedorahosted.org
To unsubscribe send an email to copr-devel-le...@lists.fedorahosted.org

Re: fedmsg structure change?

2017-01-24 Thread Pierre-Yves Chibon 
On Tue, Jan 24, 2017 at 01:36:59PM +0100, Michal Novotny wrote:
>On Tue, Jan 24, 2017 at 11:44 AM, Pierre-Yves Chibon 
>wrote:
> 
>  On Tue, Jan 24, 2017 at 11:30:33AM +0100, Michal Novotny wrote:
>  >    Yes, untested changes got into production. We are sorry. I am
>  currently
>  >    working on a fix.
> 
>  If that made it up to production, then we'll need to adjust fedmsg_meta
>  to
>  support these messages as well.
> 
>  fedmsg_meta should work for all our published messages (published and
>  stored on
>  datagrepper).
> 
>Okay, these messages have all empty body (the "msg" attribute), e.g
>`build.end`:
> 
>  {
>"source_name": "datanommer",
>"i": 3,
...
>"timestamp": 1485166967.0,
>"msg_id": "2017-db23086c-2001-496e-a59f-10b92c466ae5",
>"topic": "org.fedoraproject.prod.copr.build.end",
>"source_version": "0.6.5",
>"signature": 
> "HJEOhP93vZFUk3cjIzggxZqIT+IRaLpKF/t21Kn0AcQ9B1VJEe+myerAAJMZfuXppGQqsFyzcPFx\nu+9p7geI5NqNxnN+diUXNlxbXN9/VN0X3vX7U4mbc0/zLyGcbKWIn/pcskbM5qYC2lJHLov0pMwq\nrbq/B0N3CxBL2og0Fj8=\n",
>"msg": {}
>  }
> 
>Does fedmsg_meta need to be adjusted even so?

Yeah, fedmsg_meta should be able to process all the messages stored in
datagrepper, otherwise it breaks datagrepper and a few other apps.

So we'll end up with some not so useful message: "a build ended in copr" but we
should add them.

The easiest for this type of changes is to use test-driven-development, add the
new message to the test, fill in the expectations and then adjust the processor
to handle those cases.

Pierre
___
copr-devel mailing list -- copr-devel@lists.fedorahosted.org
To unsubscribe send an email to copr-devel-le...@lists.fedorahosted.org

Re: fedmsg structure change?

2017-01-24 Thread Michal Novotny 
Otherwise, in the fix that I am going to apply tomorrow, there will some
additional fields for chroot.start and build.start messages.

For chroot.start:
- status (int)
- version (str)

For build.start:
- status (int)
- version (str)
- chroot (str)

The old fields will be all there with the same type. Only some new fields
will be added. I hope that does not represent a problem for fedmsg_meta or
anybody. I am aware that an ideal way would be to just leave the old
message interface as it was but the code would get very ugly.  Of course, I
will do it if needed.

clime


On Tue, Jan 24, 2017 at 1:36 PM, Michal Novotny  wrote:

>
>
> On Tue, Jan 24, 2017 at 11:44 AM, Pierre-Yves Chibon 
> wrote:
>
>> On Tue, Jan 24, 2017 at 11:30:33AM +0100, Michal Novotny wrote:
>> >Yes, untested changes got into production. We are sorry. I am
>> currently
>> >working on a fix.
>>
>> If that made it up to production, then we'll need to adjust fedmsg_meta to
>> support these messages as well.
>>
>> fedmsg_meta should work for all our published messages (published and
>> stored on
>> datagrepper).
>>
>
>
> Okay, these messages have all empty body (the "msg" attribute), e.g
> `build.end`:
>
> {
>   "source_name": "datanommer",
>   "certificate": 
> "LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUVUakNDQTdlZ0F3SUJBZ0lDQVBZd0RRWUpL\nb1pJaHZjTkFRRUZCUUF3Z2FBeEN6QUpCZ05WQkFZVEFsVlQKTVFzd0NRWURWUVFJRXdKT1F6RVFN\nQTRHQTFVRUJ4TUhVbUZzWldsbmFERVhNQlVHQTFVRUNoTU9SbVZrYjNKaApJRkJ5YjJwbFkzUXhE\nekFOQmdOVkJBc1RCbVpsWkcxelp6RVBNQTBHQTFVRUF4TUdabVZrYlhObk1ROHdEUVlEClZRUXBF\nd1ptWldSdGMyY3hKakFrQmdrcWhraUc5dzBCQ1FFV0YyRmtiV2x1UUdabFpHOXlZWEJ5YjJwbFkz\nUXUKYjNKbk1CNFhEVEUwTURReU16RTBNamsxTVZvWERUSTBNRFF5TURFME1qazFNVm93Z2R3eEN6\nQUpCZ05WQkFZVApBbFZUTVFzd0NRWURWUVFJRXdKT1F6RVFNQTRHQTFVRUJ4TUhVbUZzWldsbmFE\nRVhNQlVHQTFVRUNoTU9SbVZrCmIzSmhJRkJ5YjJwbFkzUXhEekFOQmdOVkJBc1RCbVpsWkcxelp6\nRXRNQ3NHQTFVRUF4TWtZMjl3Y2kxamIzQnkKTFdKbExtTnNiM1ZrTG1abFpHOXlZWEJ5YjJwbFkz\nUXViM0puTVMwd0t3WURWUVFwRXlSamIzQnlMV052Y0hJdApZbVV1WTJ4dmRXUXVabVZrYjNKaGNI\nSnZhbVZqZEM1dmNtY3hKakFrQmdrcWhraUc5dzBCQ1FFV0YyRmtiV2x1ClFHWmxaRzl5WVhCeWIy\ncGxZM1F1YjNKbk1JR2ZNQTBHQ1NxR1NJYjNEUUVCQVFVQUE0R05BRENCaVFLQmdRQ2UKREs5VFQy\nM05BdTZPWTVGMnVVNHpMRW9Ld2k1RnRRTU5jVWV5eDdmOHJxMUZXaUxDWHBjWFhpU2tzUE1XV1NM\nWQo5SHNoa1pvM3ZjMHFSRXVBWDNweWRuM2VFRDA0UExrUmRlaWpvSXA5L0Y2YlZ3MmlLMDdXRmc5\nU2MwNlRsKzhSCld1RHNaeTQ1SVJKYXhCRTlJaHBYL0x2Y2JnQ1cvZmVHVGp5WG1iRHd0UUlEQVFB\nQm80SUJWekNDQVZNd0NRWUQKVlIwVEJBSXdBREF0QmdsZ2hrZ0JodmhDQVEwRUlCWWVSV0Z6ZVMx\nU1UwRWdSMlZ1WlhKaGRHVmtJRU5sY25ScApabWxqWVhSbE1CMEdBMVVkRGdRV0JCUm5lNTg0d3Bs\nWGYrZVE2K25zSTZCbm5BNENaRENCMVFZRFZSMGpCSUhOCk1JSEtnQlJyUUZyNUVnaUpXZWRaNVFY\nMUFoMEtUbjhVQUtHQnBxU0JvekNCb0RFTE1Ba0dBMVVFQmhNQ1ZWTXgKQ3pBSkJnTlZCQWdUQWs1\nRE1SQXdEZ1lEVlFRSEV3ZFNZV3hsYVdkb01SY3dGUVlEVlFRS0V3NUdaV1J2Y21FZwpVSEp2YW1W\namRERVBNQTBHQTFVRUN4TUdabVZrYlhObk1ROHdEUVlEVlFRREV3Wm1aV1J0YzJjeER6QU5CZ05W\nCkJDa1RCbVpsWkcxelp6RW1NQ1FHQ1NxR1NJYjNEUUVKQVJZWFlXUnRhVzVBWm1Wa2IzSmhjSEp2\nYW1WamRDNXYKY21lQ0NRRGpVQjVIVHhjZVJUQVRCZ05WSFNVRUREQUtCZ2dyQmdFRkJRY0RBakFM\nQmdOVkhROEVCQU1DQjRBdwpEUVlKS29aSWh2Y05BUUVGQlFBRGdZRUFVazNlbjBYUXpDQm5IUlh4\nZDhyOHp2ZFAwVURvbEpiUysyTEl3Z3NDClJDMnNkZ1UwNGdFblYxdFpVTjNydEk1SzQ2MnpKT0JQ\nOFhQd3h4eUZMN1lOYmVtWTgyTG52Y1pHdzliMGdxTDMKdHNKbzllSFV5SXBZMG93TlVKdzgzU1Ax\neFJvb3NwVGJRK3BsNm9qdjVPNVpGZ1lBUG1yckRWZ0M4a2gzRlp4Rgp0SWc9Ci0tLS0tRU5EIENF\nUlRJRklDQVRFLS0tLS0K\n",
>   "i": 3,
>   "timestamp": 1485166967.0,
>   "msg_id": "2017-db23086c-2001-496e-a59f-10b92c466ae5",
>   "topic": "org.fedoraproject.prod.copr.build.end",
>   "source_version": "0.6.5",
>   "signature": 
> "HJEOhP93vZFUk3cjIzggxZqIT+IRaLpKF/t21Kn0AcQ9B1VJEe+myerAAJMZfuXppGQqsFyzcPFx\nu+9p7geI5NqNxnN+diUXNlxbXN9/VN0X3vX7U4mbc0/zLyGcbKWIn/pcskbM5qYC2lJHLov0pMwq\nrbq/B0N3CxBL2og0Fj8=\n",
>   "msg": {}
> }
>
> Does fedmsg_meta need to be adjusted even so?
> clime
>
>
>>
>> Pierre
>>
>>
>> >On Tue, Jan 24, 2017 at 11:02 AM, Pierre-Yves Chibon <
>> pin...@pingoured.fr>
>> >wrote:
>> >
>> >  Hi all,
>> >
>> >  We have recently been receiving emails about wrongly formatted
>> fedmsg
>> >  message
>> >  coming from copr in stg.
>> >  Has there been any changes made to the structure of the messages
>> sent in
>> >  stg?
>> >
>> >  To give you an idea, this is what we received:
>> >  Traceback (most recent call last):
>> >  Â  File "/usr/lib/python2.7/site-packages/moksha/hub/api/consumer.
>> py",
>> >  line 191, in _work
>> >  Â  Â  self.consume(message)
>> >  Â  File "/usr/lib/python2.7/site-packa
>> ges/fmn/consumer/consumer.py",
>> >  line 89, in consume
>> >  Â  Â  self.work(session, raw_msg)
>> >  Â  File "/usr/lib/python2.7/site-packa
>> ges/fmn/consumer/consumer.py",
>> >  line 105, in work
>> >  Â  Â  msg['msg']['owner'] in self.ignored_copr_owners:
>> >  KeyError: 'owner'
>> >
>> >  The message being:
>> >  {'body': {u'username': u'copr', u'certificate':
>> >