[externals] Modify context construction to support direct invocation for SubprocessExecutionResource #15984
Conversation
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
| @@ -54,8 +55,8 @@ def build_external_execution_context( | |||
| _convert_time_window(partition_time_window) if partition_time_window else None | |||
| ), | |||
| run_id=context.run_id, | |||
| job_name=context.job_def.name, | |||
There was a problem hiding this comment.
how is a BoundOpExecutionContext getting here in the first place? Shouldn't it always be bound by the time it gets to user space?
There was a problem hiding this comment.
Confused by your question, it sounds like it should say "how is an UnboundOpExecutionContext...
BoundOpExecutionContext is the name of the passed class as context when directly invoking, it has nothing to do with the externals work.
There was a problem hiding this comment.
Yeah I got confused and meant UnboundOpExecutionContext because it is confusing that we have both BoundOpExecutionContext and OpExecutionContext as non-abstract classes
smackesey
force-pushed
the
sean/externals-support-direct-invocation
branch
from
dcdbddf to
bdc8bea
Compare
|
Deploy preview for dagster-docs ready! Preview available at https://dagster-docs-lyfpo2utz-elementl.vercel.app Direct link to changed pages: |
…for SubprocessExecutionResource
smackesey
force-pushed
the
sean/externals-support-direct-invocation
branch
from
bdc8bea to
1eb11d3
Compare
…for SubprocessExecutionResource (dagster-io#15984) ## Summary & Motivation This required a few minor supporting changes to `OpExecutionContext` and `BoundOpExecutionContext`. ## How I Tested These Changes New unit test.
Summary & Motivation
This required a few minor supporting changes to
OpExecutionContextandBoundOpExecutionContext.How I Tested These Changes
New unit test.