-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Direct invocation asset execution context #18549
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Dec 6, 2023
Merged
jamiedemaria
force-pushed
the
jamie/asset-di
branch
from
December 6, 2023 23:35
ee347b4
to
1247272
Compare
jamiedemaria
changed the base branch from
jamie/two-contexts
to
jamie/asset-context-deprecation-setup
December 6, 2023 23:35
jamiedemaria
force-pushed
the
jamie/asset-context-deprecation-setup
branch
from
December 6, 2023 23:42
e7e7de8
to
b8f9c84
Compare
jamiedemaria
force-pushed
the
jamie/asset-di
branch
from
December 6, 2023 23:42
1247272
to
ca5f755
Compare
jamiedemaria
force-pushed
the
jamie/asset-context-deprecation-setup
branch
from
December 7, 2023 19:56
b8f9c84
to
b02eb83
Compare
jamiedemaria
force-pushed
the
jamie/asset-di
branch
from
December 7, 2023 19:56
ca5f755
to
b845df7
Compare
jamiedemaria
force-pushed
the
jamie/asset-context-deprecation-setup
branch
from
December 7, 2023 20:02
b02eb83
to
6e4cf32
Compare
jamiedemaria
force-pushed
the
jamie/asset-di
branch
from
December 7, 2023 20:02
b845df7
to
11d446a
Compare
jamiedemaria
force-pushed
the
jamie/asset-context-deprecation-setup
branch
from
December 7, 2023 20:19
6e4cf32
to
4b26c05
Compare
jamiedemaria
force-pushed
the
jamie/asset-di
branch
2 times, most recently
from
December 7, 2023 20:29
8fcb99f
to
a060f2b
Compare
jamiedemaria
force-pushed
the
jamie/asset-context-deprecation-setup
branch
from
December 7, 2023 20:53
41e0231
to
2b008bc
Compare
jamiedemaria
force-pushed
the
jamie/asset-di
branch
from
December 7, 2023 20:53
a060f2b
to
8328998
Compare
jamiedemaria
force-pushed
the
jamie/asset-context-deprecation-setup
branch
from
December 7, 2023 21:26
2b008bc
to
4f92faa
Compare
jamiedemaria
force-pushed
the
jamie/asset-di
branch
2 times, most recently
from
December 7, 2023 21:35
42a4ac0
to
f1a239f
Compare
jamiedemaria
force-pushed
the
jamie/asset-context-deprecation-setup
branch
from
December 8, 2023 19:33
f29383b
to
914a6a6
Compare
jamiedemaria
force-pushed
the
jamie/asset-di
branch
from
December 8, 2023 19:33
f1a239f
to
3ca5a82
Compare
jamiedemaria
force-pushed
the
jamie/asset-context-deprecation-setup
branch
from
December 8, 2023 20:32
914a6a6
to
fc6cef4
Compare
jamiedemaria
force-pushed
the
jamie/asset-di
branch
from
December 8, 2023 20:32
3ca5a82
to
f24d391
Compare
jamiedemaria
commented
Dec 8, 2023
python_modules/dagster/dagster/_core/execution/context/invocation.py
Outdated
Show resolved
Hide resolved
jamiedemaria
force-pushed
the
jamie/asset-context-deprecation-setup
branch
from
January 29, 2024 18:16
6b387ff
to
372b752
Compare
jamiedemaria
force-pushed
the
jamie/asset-di
branch
from
January 29, 2024 18:16
7581aea
to
f1a29e0
Compare
jamiedemaria
force-pushed
the
jamie/asset-context-deprecation-setup
branch
from
January 29, 2024 19:44
372b752
to
d967f43
Compare
jamiedemaria
force-pushed
the
jamie/asset-di
branch
2 times, most recently
from
January 29, 2024 20:03
f05054c
to
03fc95f
Compare
Deploy preview for dagster-docs ready! Preview available at https://dagster-docs-hbembp2r1-elementl.vercel.app Direct link to changed pages: |
alangenfeld
approved these changes
Jan 30, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, looks clean to me
Base automatically changed from
jamie/asset-context-deprecation-setup
to
master
January 30, 2024 20:18
jamiedemaria
force-pushed
the
jamie/asset-di
branch
from
January 30, 2024 20:19
3f60787
to
63e5bc6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary & Motivation
This PR introduces a direct invocation AssetExecutionContext. The underlying
op_execution_context
is aDirectOpExecutionContext
. Methods onAssetExecutionContext
that have their own implementations, rather than passing through to a method on the op execution context, and access values not available during direct invocation will need to be re-implemented on theDirectAssetExecutionContext
(but there are none of those in this PR). The bound/unbound state of this context is maintained on the underlying op contextIntroduces a parent class for
DirectOpExecutionContext
andDirectAssetExecutionContext
so we can do type checking in the direct invocation code pathHow I Tested These Changes