I'm not quite sure about the message syntax. Currently, I'm using a syntax like this:

{ 'policies_satisified': False, 'decision_context': 'bodhi_update_push_stable', 'unsatisfied_requirements': [ { 'testcase': 'dist.abicheck', 'item': { 'item': nvr, 'type': 'koji_build' }, 'type': 'test-result-missing' }, { 'testcase': 'dist.upgradepath', 'item': { 'item': nvr, 'type': 'koji_build' }, 'type': 'test-result-missing' } ], 'summary': '2 of 3 required tests not found', 'product_version': 'fedora-26', 'subject': [ { 'item': nvr, 'type': 'koji_build' } ], 'applicable_policies': ['taskotron_release_critical_tasks'] }

This looks good to me.

Do these only get published when the satisfaction changes from False to True or from True to False? Or do they get published for every waiver and every result?

Maybe it could be interesting to have a msg that contains the old decision and the new decision so that a listener could act on the delta, somehow.

Yeah, I'm going to do it in another patch. In this patch, they get published for every result and every waiver. What should the best syntax be for containing the old decision and the new decision? I guess we can use a new key 'pre_decsion'?

Sure. I'd vote for just using previous instead of pre_decision.

Sounds good to me, thanks.

Damn it! I've forgotten to commit a directory including the consumers in this patch. The worst thing is I can't find it from my local git repo. So please do not review and I will post a new update when I find time.

Any luck finding it @mjia?

Sorry, I just come back from PTO. I will start reworking on this PR.

Okay, I have managed to bring these missing consumers back and this PR is ready now for another round.

As I understand, if the results and waivers are cached as described in PR#84, I should be able to use these cached results and waivers to get the old decision. Correct me if I am wrong, :-P

Interesting. I see this is re-implementing the logic of the /decision endpoint.

What do you think about having this make an actual HTTP POST to the greenwave web interface? That way, we could keep only one stretch of code that handles computing a decision.

Well, yes. :)

In general, this looks like a good start. A few things:

• Consider querying the HTTP interface of greenwave to get the decision instead of making the decision in the code here. Maybe that's too indirect.. but it could be nice to try and de-duplicate the decision logic.
• We'll have to figure out how to compute the "old decision".
• In a world where this code queries the greenwave web interface to get the decision, I imagine it making two queries. One, normal decision query, and a second decision query that specifies additionally that the decision should ignore the given result_id or waiver_id.
• Once the backend has both the old decision and the new decision (however it gets those, via its own computation, or via a function that it shares with the frontend, or via an http post call..), then it can compare the old decision and the new decision. I think it should only publish a fedmsg message if the old message is different from the new message. I.e., if there's a change.

Yeah, you are right. Somehow I was trying to avoid sending indirect requests.

@ralph, thank you very much for your review. I like the idea of how to compute the "old version".

Rebased to address the feedback and it is ready for another look, :-P

Interesting. This assumes that all of our rules will have a test_case_name, which they currently do. But will they always? We could look at abstracting this interface later. :+1: to keep it for now.

This looks awesome.

Running the tests now.

32 passed, 3 warnings in 5.97 seconds

Looks good! Merging.

We'll need to figure out how we actually deploy this in openshift (it is effectively optional, fwiw. the frontend continues to work without it.)

We'll need some kind of side container for the messaging daemon.

See also https://src.fedoraproject.org/container/greenwave/c/e29f931fb6153af4efdeda2dff74bfd96661d689?branch=master

Yeah, we can always adjust this if we need to introduce some rules that do not have a test_case_name.