Wait, we support arbitrary text for the name? I don't think thats a good idea. We should probably just be supporting lowercase alphanumeric with underscore.

Not sure if its a good practice to query the registry here and pass in the join keys. Its almost a half step. We should be pulling the feature views from the registry here, based on their names. Then we should either

• Pull the entities as you've done it here and have a helper function like run_reverse_field_mapping(feature_view, entities) which is a reversal of this method approach, or
• When we pull the FeatureViews from the registry they get a reference to the FeatureStore stored inside of them. Then we can add a method like FeatureView.get_entities() which returns its Entity objects. May need to change the previous one to FeatureView.entity_names so that it isnt confusing. Anyway, then when you run FeatureView.run_reverse_field_mapping() it will find its own entities using the reference it has to the FeatureStore class, as opposed to it being passed in.
  or
• We remove this List[str] as entities and replace it with List[Entity] so that when we do a get_feature_view() we actually get the entity details back in the first place. Its also much cleaner for end users when registering FeatureViews since they dont have to deal with string references.

Given how much time is left, I am leaning towards the first option, but its definitely technical debt.

What do you think @jklegar ?

Yeah there's definitely tech debt here - IMO the main problem is getting the feature view from the registry doesn't also get the entities from the registry and we should do something like your third option (in a separate diff). Happy to do the first option here though not sure how much it actually gets us since we need join_keys in this method further down anyway

I guess my concern is just that we are making the code base more complicated (using a method) but we're not gaining anything from that since the method acts like a function.

need join_keys in this method further down anyway

Wouldnt we be able to use this list of entities to get the join keys? I think what I am trying to avoid as much as possible is to have methods with I/O. I want to us to try and follow a more immutable/functional approach as much as we can (which I realize isnt 100% possible). So get_entities/get_feature_views etc should ideally be run only once near the start of one of our methods and then the state should just propagate down. It'll be way easier to reason about and test long term.