Page MenuHomePhabricator

bye-system-storage-1 cut file manager non-resource paths
ClosedPublic

Authored by yuhan on Nov 25 2020, 10:14 PM.

Details

Summary

in order to remove system storages, we need to cut file manager from it first.

In 0.9.0, we've moved the file_manager to be specified via resources, so the usage of it inside a solid would be context.resources.file_manager instead of context.file_manager

Test Plan

bk

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.Nov 25 2020, 10:31 PM
Harbormaster failed remote builds in B21810: Diff 26491!
Harbormaster returned this revision to the author for changes because remote builds failed.Nov 26 2020, 12:56 AM
Harbormaster failed remote builds in B21815: Diff 26497!
Harbormaster returned this revision to the author for changes because remote builds failed.Nov 26 2020, 1:20 AM
Harbormaster failed remote builds in B21816: Diff 26498!
Harbormaster returned this revision to the author for changes because remote builds failed.Nov 30 2020, 7:04 PM
Harbormaster failed remote builds in B21874: Diff 26562!
Harbormaster returned this revision to the author for changes because remote builds failed.Nov 30 2020, 7:59 PM
Harbormaster failed remote builds in B21881: Diff 26568!
Harbormaster returned this revision to the author for changes because remote builds failed.Nov 30 2020, 11:44 PM
Harbormaster failed remote builds in B21898: Diff 26592!
Harbormaster returned this revision to the author for changes because remote builds failed.Dec 1 2020, 12:13 AM
Harbormaster failed remote builds in B21900: Diff 26594!
Harbormaster returned this revision to the author for changes because remote builds failed.Dec 1 2020, 12:44 AM
Harbormaster failed remote builds in B21901: Diff 26595!
yuhan requested review of this revision.Dec 1 2020, 1:30 AM
schrockn requested changes to this revision.Dec 1 2020, 8:38 PM

given that we didn't go through a deprecation cycle, I think that property should at a minimum throw a helpful error message

python_modules/dagster/dagster/core/execution/context/step.py
18

a bit unfortunate to not go through a deprecation cycle

This revision now requires changes to proceed.Dec 1 2020, 8:38 PM
schrockn added inline comments.
python_modules/dagster/dagster/core/execution/context/step.py
19–22

nit: NotImplementedError nicer than bare Exception

python_modules/dagster/dagster/core/execution/context/system.py
160–163

same same

This revision is now accepted and ready to land.Dec 1 2020, 10:44 PM
yuhan marked 2 inline comments as done.

NotImplementedError

introduce DagsterInvalidPropertyError as

  • there might be more cases like this
  • raise NotImplementedError gives false positive pylint error and suppressing it (i.e. disable=abstract-method) seems too much

excellent i could use that exception in my stack too