Hi @cverna, Jenkins build failed because new dependencies are not present in the Jenkins slave image and warnings came from upgraded Flake8. Hopefully #293 and #294 can solve or mitigate those issues.

pylint gives this warnings:
waiverdb/events.py:102:31: W1202: Use % formatting in logging functions and pass the % parameters as arguments (logging-format-interpolation)
waiverdb/events.py:105:31: W1202: Use % formatting in logging functions and pass the % parameters as arguments (logging-format-interpolation)

@cverna Please rebase again, #294 has been merged, so it should eliminate the pylint's woof-woof. :)

The other stuff (W1202, mentioned by @gnaponie ) needs to be fixed in your PR.

@gnaponie and @fivaldi Ok that should be fixed and rebased :fireworks:

Currently fails in these tests:

12:13:04 tests/test_events.py::test_publish_new_waiver_with_fedmsg FAILED         [ 89%]
12:13:04 tests/test_events.py::test_publish_new_waiver_with_fedmsg_for_proxy_user FAILED [ 90%]

on:

12:13:04 >                   assert msg == expected
12:13:04 E                   AssertionError

in mock_sends(*expected_messages)

12:13:04 expected_messages = (Message(id='2673684e-d975-496b-b9d3-cbfa29ea2664', topic='waiver.new', body=OrderedDict([('id', 183), ('subject_type'...on', 'something'), ('waived', True), ('comment', 'This is a comment'), ('timestamp', '2019-03-05T11:13:03.784722')])),)
12:13:04 mock_cache = <MagicMock name='_session_cache' id='140624069483320'>
12:13:04 messages = [Message(id='6aaebea8-a94b-4a46-b7b2-2e82fe551c37', topic='waiverdb.waiver.new', body=OrderedDict([('id', 183), ('subj...ion', 'something'), ('waived', True), ('comment', 'This is a comment'), ('timestamp', '2019-03-05T11:13:03.784722')]))]

Seems like an issue with list/tuple when comparing...

Ok I have updated the test :smile:

It seems good now +1

Thank you for this change!

fedora-messaging only supports AMQP 0.9, which we don't have available internally (STOMP or AMQP 1.0).
Can we make it so it uses either depending on configuration?

FWIW, we are planning to look into extending fedora-messaging to also handle AMQP 1.0.

cc @mikeb, @mprahl

@lucarval isn't that already in with https://pagure.io/fork/cverna/waiverdb/blob/4ecfc311e9243e1ac1d65b7412d9fa671d2a5b39/f/waiverdb/events.py#_68 ?

@pingou, you're right. Sorry for the confusion. Change LGTM.

Hi @cverna, can you do a rebase to include #294 for eliminating the W504 flake8 warning?

BTW, subsequent Jenkins builds will fail with RPM build errors because new RPM dependencies can't be installed automatically during a Jenkins job run (we don't have root access to the Jenkins slave pod on OpenShift). That is a known limitation of the WaiverDB CI/CD pipeline. I have run oc start-build waiverdb-premerge-jenkins-slave --commit=pull/291/head for you to rebuild the Jenkins slave image with new dependencies introduce by this PR. So the Jenkins build should be green after you rebase.

Just did the rebase and the CI seems to be happy from what I can see

Ok latest rebase made the CI to fail :cry:

I think it is caused by a network failure:
fatal: unable to access 'https://pagure.io/waiverdb.git/': Could not resolve host: pagure.io

Let me trigger it again manually.

I think it is caused by a network failure:
fatal: unable to access 'https://pagure.io/waiverdb.git/': Could not resolve host: pagure.io
Let me trigger it again manually.

:thumbsup: that fixed the issue.

Any chance to have this merged soon ? We would like to be able to test it in staging before the end of week if possible.

+1

@rayson Thanks. Any chance to have a container built with a dedicated tag that we could use in staging to test this ?

I am not sure what is your process for releases :-)

@cverna, yes, we can run waiverdb-postmerge job with WAIVERDB_GIT_REF parameter set to the Git tag you want to build and WAIVERDB_DEV_IMAGE_TAG parameter set to the image tag.

For example:

oc start-build waiverdb-postmerge -e WAIVERDB_GIT_REF=my-git-tag -e WAIVERDB_DEV_IMAGE_TAG=my-image-tag

Then you can pull the image from quay.io/factory2/waiverdb:my-image-tag