Page MenuHomeElementl

fix import dagster intermediate storage deprecation warning #3478
ClosedPublic

Authored by yuhan on Jan 6 2021, 2:46 AM.

Details

Summary

https://github.com/dagster-io/dagster/issues/3478

depends on D5834 (use mem_object_manager as the default rather than the adapted mem intermediate storage)

Test Plan

unit + import dagster dones't produce deprecation warnings
not sure why https://sourcegraph.com/github.com/dagster-io/dagster/-/blob/python_modules/dagster/dagster_tests/core_tests/test_import.py isn't working

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Jan 6 2021, 3:04 AM
Harbormaster failed remote builds in B23688: Diff 28810!
Harbormaster returned this revision to the author for changes because remote builds failed.Jan 7 2021, 11:55 PM
Harbormaster failed remote builds in B23883: Diff 29036!
Harbormaster returned this revision to the author for changes because remote builds failed.Jan 8 2021, 1:38 AM
Harbormaster failed remote builds in B23892: Diff 29046!
Harbormaster returned this revision to the author for changes because remote builds failed.Jan 8 2021, 8:57 PM
Harbormaster failed remote builds in B23947: Diff 29111!
Harbormaster returned this revision to the author for changes because remote builds failed.Jan 9 2021, 1:06 AM
Harbormaster failed remote builds in B23999: Diff 29182!
yuhan requested review of this revision.Jan 9 2021, 9:15 AM

If I understand correctly, this will push the deprecation warning from pipeline definition time to pipeline run time? Not a big problem, but if there's an easy way to surface a warning earlier, that could be nice.

This revision is now accepted and ready to land.Jan 9 2021, 5:48 PM

@sandyryza that's right. the issue was caused by fs_intermediate_storage and mem_intermediate_storage in the dagster/__init__.py which call the IntermediateStorageDefinition path. So if we want to warn at definition time, the way i could think of is to pass in a flag like do_not_warn to IntermediateStorageDefinition so fs_intermediate_storage and mem_intermediate_storage won't fire deprecation warnings when import dagster. but i felt that was a bit hacky so i ended up moving it to pipeline run time warning