This address that: https://github.com/fedora-infra/bodhi/issues/3089

Why insert duplicate items to verbose_* lists in the first place?

BTW, retrieve_latest method doesn't use cache so the duplicate invocation is needlessly expensive.

I'm not sure you can actually know they are the same element until you make the request...
We could check the subject_identifier. But we could also have 2 waivers for the same subject_identifier (different product versions). Can we have 2 waivers for the same subject_identifier and product_version but different subject_type? I dunno.

We could for sure don't repeat the request if all 3 are the same. But I think the user shouldn't make this kind of request in the first place if the subjects are the same element... and also: "subject" parameter is deprecated and people should just stop using it.

I think this is a OK quick fix. The ultimate goal should be stop supporting "subject" in favor of "subject_identifier" and "subject_type". But we have to make a major release of the API for that.

OK, but can you use set instead of list? The "removing duplicated elements" code doesn't look very nice.

I agree with @lholecek to use set instead of list. If you need to preserve order you can use ordered_set.

You cannot set() on a list of dict because dict is not hash-able. I've put another version. Is it fine now?

Oh, I forgot that dict isn't hashable. Otherwise this looks much better.

+1

return list({item['id']: item for item in items}.values())

I like this approach as well. It's a lot easier to read.

All comments should be addressed and conflicts resolved.
@mprahl @lholecek another review? Thank you.

I can see trailing underscore in r_ variable name in other already existing tests, but it should not be used for new tests, or does the trailing _ have any special meaning?

It should/could be possible cleared for all other tests in a separate PR.

@gnaponie, looks good, just a minor comment there... Tests are passing for the PR?

Mmm not sure why it is like that in other tests. I prefer "r_" instead of "r" because if you use the debugger (pdb) and try to check "r" it actually "returns" (because it is a debugger command) and that's annoying...

@gnaponie, looks good, just a minor comment there... Tests are passing for the PR?

I've tried once again and they pass on my local machine.

+1

+1

Hint: you can use p r in the debugger

Commit ea4a93e8 fixes this pull-request