- make AssetStoreContext take all the flat info we offer as properties, so mocking would be easy
- introduce a static method AssetStoreContext.construct to parse the info from internal representations and build the context
I don't think we need to include execution plan in the initialization, but rather pre-compute what we need from it. I think this will make testing easier as well.
updated. the context should be straightforward to mock now.
@cdecarolis i was thinking maybe we could do the same in a new helper execution_plan.construct_asset_store_context and feed the step there.
i didn't include these^ in this diff because it'd be easier to test that method in your diff.
Nitpick: are these parentheses needed?
For the slack digest pipeline, we'll need the solid's tags. If we're providing both those and the name, IMO best to just provide the whole solid definition.
Do we want to expose this method to users? If not, maybe preferable to put it elsewhere. (Maybe that's what you're already suggesting in your comment?)
cool, that makes sense. Do you think it makes more sense to include execution_plan.construct_asset_store_context here, or in my subsequent diffs, due to it being easier to test there as well?
ok i think i was just being lazy.. but then i realized i needed to test it anyways, so i wrote that in this diff.
i moved it to SystemStepExecutionContext.for_asset_store and ExecutionPlan.construct_asset_store_context. it also made my life so much easier.