Page MenuHomePhabricator

add mem_asset_store, support different asset stores per solid
ClosedPublic

Authored by sandyryza on Oct 30 2020, 5:10 PM.

Details

Summary

previously, if a solid didn't have the asset store that its upstream dependencies did, the asset
store resource wouldn't be available to it for reading

Test Plan

added tests

Diff Detail

Repository
R1 dagster
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Oct 30 2020, 6:14 PM
Harbormaster failed remote builds in B20532: Diff 24894!
Harbormaster returned this revision to the author for changes because remote builds failed.Oct 30 2020, 7:08 PM
Harbormaster failed remote builds in B20590: Diff 24952!

this looks good! is the plan to allow users to specify an asset store once on pipeline and so all solids will be using the same asset store?

python_modules/dagster/dagster_tests/core_tests/storage_tests/test_asset_store.py
133–146

how is it different from the test_different_asset_stores?

162

nit: can we do assert execute_pipeline(asset_pipeline).success - assuming we want an assertion failure not an uncaught error

is the plan to allow users to specify an asset store once on pipeline and so all solids will be using the same asset store?

that's what I was thinking - via the "default_asset_store" key

python_modules/dagster/dagster_tests/core_tests/storage_tests/test_asset_store.py
133–146

hmm yeah, they're basically the same. I'll consolidate

162

good point

python_modules/dagster/dagster/core/storage/asset_store.py
65

Nit: shouldn't this be InMemoryAssetStore to keep naming schema consistent?

This revision is now accepted and ready to land.Mon, Nov 2, 8:01 PM