Mmm I don't like that. We did an effort in the past to remove hard-coded subject types in Greenwave and make it more easily to have "whatever" subject type. I thought it was a great thing and it was sad that we couldn't remove them even more.
This looks like going backwards to me...

This does not hard-code anything. It just moves the specialized functionality of subject types to separate submodules so it's easier to create another special subject types.

Having to handle subject types in various places in code doesn't scale well.

It creates a whole file and class for these subject types, it is hard-coded IMHO.

It creates a whole file and class for these subject types, it is hard-coded IMHO.

That doesn't sound like hard-coding. Not sure what you mean by it. Can you explain?

Currently, if you want to create a special handling for a subject type you probably need to change behavior in policies and rules (well, and couple of other places as well). But why should the policies and rules know how to handle special subject type? This basically means having if subject_type == 'koji_build' and other branches everywhere.

The change is to move the logic to a single submodule (subjects) so the other code can just directly query the specialized subject objects.

Later this would allow us to handle the specialized classes as plugins, e.g. Fedora instances wouldn't require to load RedHatModule (this is something we had in PDC as well).

That doesn't sound like hard-coding. Not sure what you mean by it. Can you explain?
You even have a variable with this: is_koji_build
From wikipedia: Hard coding is the software development practice of embedding data directly into the source code. It seems hard-coded to me.

At the moment you can have any kind of subject type. The problem is that we have some special cases that have to be handled in the code due to the fact that before the subject types were hard-coded and we had to keep backward compatibility.
I'd like to remove completely special cases, and not standardize them on a class and file.
For example, if the user uses "brew-build" instead of "koji_build" I think it's their own problem. Greenwave shouldn't care that brew == koji. This was done for backward compatibility
Maybe in next major version we should just announce this is not going to be supported any more and drop it.
Remote rules accepted subject types should be in a variable in the configuration.

OK, but this is not only about brew-build ~ koji_build (thought with this change it will be easier to remove it). This change replaces on various places: the subject item/type serialization, creating resultsdb query, getting product version (unfortunately, not completely) and getting the name to check blacklist/excluded_packages.

Remote rules accepted subject types should be in a variable in the configuration.

Hmm, maybe we can have everything in configuration, but not sure how simple it would be.

I feel like this could be carelessly used. Can we just look for the keys we care about?

for k in ('type', 'identifier'):
  v = data.get(k)
  if v:
    unpacked[k] = _unpack_value(v)

UPDATE: Disregard this comment. I see now that the whole point of this refactor is to isolate which keys/attributes we care about.

PR version 2 with subject type configuration in YAML: #493

Use doublequotes to avoid escapes

Would rather use comprehension here:
~~~~
unpacked = {k: _unpack_value(v) for k, v in data.items()}
~~~~~

Would rather use comprehension here

Good idea. I'll fix it in the #493 and close this PR.

Use doublequotes to avoid escapes

This is just a moved code, but I'll change it as suggested.

Use doublequotes to avoid escapes

This is just a moved code, but I'll change it as suggested.

Actually, let's use "Could not" instead.