Looks nice and simple :)

+1

Do we want to restrict to 404 here?
What about making this more flexible/resilient and use if not response.ok ?

Isn't that risky? Nonexistent Bodhi updates will always be 404. If we make more flexible we could ignore other errors. I would leave "404".

Yeah, just to be safe, I would handle only 404 this way.

Another possibility is too handle old API requests without asking for Bodhi updates.

Since you used to not rely on bodhi at all, to keep backward compatibility you need to be able to handle situations such as: network glitches, bodhi temporary down...
What would be the risk in ignoring more than just the 404?

Note: you could always add something to the logs saying "Ignoring bodhi because it returned a status code XXX"

I've not digged into the code but I just saw the name of this method and the change below, we still retrieve waivers for the old type of requests, right?

Since you used to not rely on bodhi at all, to keep backward compatibility you need to be able to handle situations such as: network glitches, bodhi temporary down... What would be the risk in ignoring more than just the 404? Note: you could always add something to the logs saying "Ignoring bodhi because it returned a status code XXX"

I've changed it to handle any error and log a warning.

I've not digged into the code but I just saw the name of this method and the change below, we still retrieve waivers for the old type of requests, right?

Yes, we do.

ok for me

I've not digged into the code but I just saw the name of this method and the change below, we still retrieve waivers for the old type of requests, right?

Yes, we do.

Thanks for the confirmation :)

I've changed it to handle any error and log a warning.

Looks good to me :)

Hmm. This looks fine but I think it should only be for the backwards compatibility API with subject.

If I am using the new API to ask explicitly about a particular Bodhi update, it should really be an error if I ask about a Bodhi update that doesn't exist.

It's only in the old API where the subject parameter could contain any extra random items which would just be ignored.

I will file an issue so we can fix it up later.

Hmmmmm... thinking about it some more... are we sure this does what we want?

This effectively means that a decision about a Bodhi update that does not exist, will be always satisfied. Previously Greenwave would also look at all the builds you gave it and consider the policy for those, but it no longer does that if it sees a bodhi_update item in the list. So this is not totally backwards compatible.

In general I am not sure it makes sense to send queries to prod Greenwave about updates which only exist in a local Bodhi dev environment, a better approach is probably to run a local Greenwave configured to point at the local Bodhi...

In general I am not sure it makes sense to send queries to prod Greenwave about updates which only exist in a local Bodhi dev environment

I'd answer that it used to work :)

But I can understand the reasoning and if that's what you want to do, we'll have to find how we can deal with it.

@pingou can you give some more info about the decisions you are requesting? Where are they from, what are they for, and which Bodhi instance is it referring to if not the prod one?

Is this for local development environments, or is it some other actual service which needs correct answers? Does it matter that now Greenwave will just always give "no test required" whereas before it would give a halfway-correct answer?

If you have real builds but not a real update, maybe the script should be asking for decisions about each build directly instead?

Or if it's just dev environments where the actual response from Greenwave doesn't really matter, maybe we can make it easier for you to run your own copy of Greenwave instead of hitting prod?

We can help to figure out the best way to keep this working for you, but I have no idea what script we are talking about right now so it's hard to know what is the best option.

It's not a script, it's a local bodhi instance that I run to work on the integration between greenwave and bodhi.

While it's "fine" if greenwave always returns "No tests required", not having it do that in stg (where the rules are configured to enforce some checks) would be nice as it allows to mimic a situation that is closer to production.
Worst case, now that we can query greenwave via bodhi, we can also just modify the data returned by greenwave directly in bodhi, making it easier to "fake" real data, so there are ways around for us :)