Unreachable code

TestInvalidGatingYaml would be more consistent with the other names.

... It should actually be just InvalidGatingYaml.

Test might be not needed in this class name, MisconfiguredRemoteRule should be just fine. (Analogous to BlacklistedInPolicy further down.)

:+1: to the general direction. :) Lukas' comments should be addressed before we merge.

Rebased. It should be fine now.

Can you move the log warning from validate_policies() here? It would be actually better if some specific messages are available in the decision response instead of in log somewhere.

If this is really need then more specific message would be nice. E.g.

"Error parsing gating.yaml for package {}: {}".format(pkg_name, e)

It should be fine now.
I'll wait also for @dcallagh's opinion.

The 'item' key is only for backwards compatibility so let's not add it in this new class. Instead it should have 'subject_type' and 'subject_identifier'.

Also I think it needs 'type': 'invalid-gating-yaml' or similar. All the other unsatisfied requirements have a 'type' like 'test-case-missing' and so forth.

Is it really right that this function returns either an empty list, or a Result instance?

Should this one actually be returning a list of one Result instance?

Hmm, this comment doesn't make sense to me... What exactly is it ensuring? Presence of what result?

It seems like we really need two separate test cases here: invalid gating.yaml, not waived => decision is unsatisfied; and invalid gating.yaml, but waived => decision is satisfied.

No need for creating temporary list, generator expression is enough, e.g. any(x for x in xs ..).

What this function will return will be also returned by the "check" method of the RemoteRule class. All the other "check" methods return some Result.
I think otherwise it is correct just to return "[]", it will be "extended" in the decision API.
If I removed the "return []" it would be in an infinite loop in the case of the DisallowedRuleError because it would never "return" anything... Not sure I explained myself :/ that was I noticed at least while I was testing the code.

I guess I can return a list with one Result, but all the other "Rule" classes are returning just the normal Result.

The comment is actually just an old copy-paste :/
I think I'll split the test into 2 separate tests with your suggestions...

Nice! Looks good now.

I still keep getting tripped by the odd interface on .check() with the different return values. I think I had the same issue with previous patches too. :-) So I will try writing a PR to see if I can make it nicer.

But in the meantime, this is :+1: from me.

+1

Commit ae2ba458 fixes this pull-request

Thank you everyone for the good feedbacks.