provide context class based on type hint

[jamiedemaria]

Summary & Motivation

Introduces the logic to determine which kind of context to provide to which kind of step.

Follows these rules:

@asset + no type annotation -> AssetExecutionContext
@asset + AssetExecutionContext annotation -> AssetExecutionContext
@asset + OpExecutionContext annotation -> OpExecutionContext 
@op + no type annotation -> OpExecutionContext 
@op + AssetExecutionContext annotation -> Error (see note)
@op + OpExecutionContext -> OpExecutionContext 

note: We error when AssetExecutionContext type hint is provided to @op because we will likely require an AssetsDefinition in the __init__ of AssetExecutionContext once we begin to split methods out (see here). It will be easier to make a change allowing the AssetExecutionContext type annotation than it will be to disallow it, so I think we should error in this case for now

For ops in graph-backed assets, we do:

@graph_asset @op + no type annotation -> OpExecutionContext
@graph_asset @op + AssetExecutionContext -> AssetExecutionContext 
@graph_asset @op + OpExecutionContext -> OpExecutionContext

The reasoning behind @graph_asset @op + no type annotation -> OpExecutionContext is best explained with an example:
Imagine you have an op that is used in both a job and a graph-backed asset

@op 
def my_fun_op(context):
   context.describe_op # this function is on the OpExecutionContext, but not on the AssetExecutionContext

@job 
def my_fun_job():
    my_fun_op()

@graph_asset 
def my_fun_asset():
    my_fun_op()

When executing my_fun_job, my_fun_op will receive an OpExecutionContext and run as expected. When materializing my_fun_asset, my_fun_op will receive an AssetExecutionContext. It will fire a deprecation warning, but still call describe_op on the internally held op_execution_context. When we deprecate the __get_attr__ behavior here, my_fun_op will instead error.

How I Tested These Changes

new unit tests

[jamiedemaria]

Current dependencies on/for this PR:

• master
  • PR [quick] use correct context type annotations in tests #16507
    • PR Make AssetExecutionContext a subclass of OpExecutionContext #16761
      • PR provide context class based on type hint #16759 👈
        • PR Add OpExecutionContext isinstance deprecation warning test #16762

This comment was auto-generated by Graphite.

[jamiedemaria]

It'd be nice to move this check to definition time, but i dont know if we can. At definition time, I dont know of a way to know if

@op 
def my_op(context: AssetExecutionContext)

is being used in a job or in a graph-backed asset

[alangenfeld]

i think this makes sense

nit: would reframe the comment before land to something like "would be nice to ... currently doesnt seem possible"

[jamiedemaria]

ok cool, will update

[alangenfeld]

i think this makes sense

nit: would reframe the comment before land to something like "would be nice to ... currently doesnt seem possible"

[alangenfeld]

note I would classify these new errors as breaking changes so, make sure it waits for the 1.5 release and get documented accordingly

[jamiedemaria]

note I would classify these new errors as breaking changes so, make sure it waits for the 1.5 release and get documented accordingly

yes, i agree. I think the last release branch before 1.5 has been cut, so this should be good to go in any time. i'll make a note to give this behavior change a callout in the changelog