Page MenuHomeElementl

Enable resource dependencies within memoizable io managers
ClosedPublic

Authored by cdecarolis on May 24 2021, 4:56 PM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 30 2022, 4:16 AM
Unknown Object (File)
Dec 19 2022, 8:03 AM
Unknown Object (File)
Dec 19 2022, 1:19 AM
Unknown Object (File)
Dec 8 2022, 11:04 AM
Unknown Object (File)
Dec 2 2022, 1:58 AM
Unknown Object (File)
Nov 28 2022, 12:57 PM
Unknown Object (File)
Nov 26 2022, 4:13 PM
Unknown Object (File)
Nov 15 2022, 11:16 AM
Subscribers
None

Diff Detail

Repository
R1 dagster
Branch
resources_output_context
Lint
Lint Passed
Unit
No Test Coverage

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.May 24 2021, 5:15 PM
Harbormaster failed remote builds in B31055: Diff 38195!

Fix resources_for_io_manager callsites

python_modules/dagster/dagster/core/execution/context/output.py
201

In what situations would this be None?

python_modules/dagster/dagster/core/execution/context/output.py
201

Approximately None

python_modules/dagster/dagster/core/execution/context/output.py
201

😂

In that case, can we make it non-optional? Then can avoid the cast below?

python_modules/dagster/dagster/core/execution/context/output.py
201

yeah honestly if its not a user facing API i think we should avoid = None ing stuff out and just be explicit at all call sites. Too many bugs from accidental omission of optional arguments

201–202

passing the whole mode and scoped builder separately feels a bit odd to me

  • maybe just take resource_defs directly instead of mode
  • maybe just take the resources directly and only allow that if step_context is None?

Address comments: get rid of defaults on get_output_context, pass resources directly to get_output_context

add test for pass both error

This revision is now accepted and ready to land.May 26 2021, 4:05 PM