[module-loaders] Make key iterator property on all asset objects

[dpeng817]

Summary & Motivation

This follows on the theme of needing to special case each different type of asset when dealing with the union list; in the case of iterating on keys, some assets have a keys parameter, some only have a key parameter. Instead, just have each asset able to return an iterable of their keys. Then, we can just call a single property and contain this complexity to the class itself.

[dpeng817]

• [module-loaders] Make key iterator property on all asset objects #26534 👈 (View in Graphite)
• [module-loaders] Split loader file into multiple different files #26533
• [module-loaders] Delete extra_source_assets param from assets_from_modules #26494 : 9 other dependent PRs (#26484 , #26498 , #26514 and 6 others)
• master

This stack of pull requests is managed by Graphite. Learn more about stacking.

[OwenKephart]

This is the PR that gave me cold feet about the downstack with_attributes PR

[OwenKephart]

Given there's currently just a single place where we need to do this, and most of this ugliness gets cleaned up by removing the include(d)_targeted_keys param, I think it makes more sense to consolidate this logic here for now

[dpeng817]

This isn't actually the only place where we need to do this, is my take. I needed to do the same thing in Airlift, and anything that operates on an unresolved Definitions object will need to do the same thing.

I think the reality is that in each of the places where we likely encounter it there's a ternary that I didn't break by doing this.

[OwenKephart]

seems like this only gets you half of the way there on the annoyance removal though, right? because you still have to itertools.chain() all of your various iterators together into a single iterator or do the dastardly double list comprehension that we currently have

if we already have a lovely map_asset_specs() method that handles the iteration and sub-iteration for you on these heterogenous lists, maybe we just supply an iterate_asset_specs() method to do the same, just without the mapping bit?

that seems a bit more general-purpose to me, and it'd let you do:

all_asset_keys = {
    spec.key
    for spec in iterate_asset_specs(self.assets_defs_specs_and_checks_defs)
}

which is a lot easier to comprehend.

we can keep it as an internal utility for now so that airlift can make use of it

[dpeng817]

I guess the only confounding factor there is source assets (since we don't support this pattern on cacheables right now anyway). and I guess checks defs would just never get any specs iterated upon, which maybe is fine?