Page MenuHomeElementl

Use cleaner NamedTuple syntax for InputContext and OutputContext
ClosedPublic

Authored by cdecarolis on Apr 15 2021, 4:50 PM.

Details

Summary

This diff switches to using the cleaner NamedTuple syntax for InputContext and OutputContext.

Two weird things:
log_manager -> log as argument to new. This is technically a breaking change for those who might be directly initting these contexts (should not affect those who are mocking them).
A bunch of casts in places where they weren't necessary before. I'm not sure why these weren't previously caught, because nothing should have changed about the optionalality of the arguments.

Test Plan

Mypy and unit

Diff Detail

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

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Apr 15 2021, 5:21 PM
Harbormaster failed remote builds in B28926: Diff 35503!
Harbormaster returned this revision to the author for changes because remote builds failed.Apr 15 2021, 5:57 PM
Harbormaster failed remote builds in B28933: Diff 35511!

being annoying but i think its worth

python_modules/dagster/dagster/core/execution/context/system.py
563

lets move these out of this file in to their in this directory

This revision now requires changes to proceed.Apr 15 2021, 8:40 PM

Moved input and output context out of system.py

oh wait - whats the testing story for IO managers? Do we expect users to create these by hand in test scenarios ? We might need to the typing.NamedTuple with __new__ version

oh wait - whats the testing story for IO managers? Do we expect users to create these by hand in test scenarios ? We might need to the typing.NamedTuple with __new__ version

That's a good question - I'm not sure of the answer. As it stands, I don't think we communicate a de-facto testing story for IO managers. If someone was directly testing an IO manager, then I'd expect that they would create the contexts by hand given that we expose them at the top level of the API.
Tldr probably: I'll stick with the __new__ stuff to be safe

Use typed namedtuple syntax to preserve constructor arguments

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