@observable_source_asset adapter to generate and SensorDefinition

[schrockn]

Summary & Motivation

This PR adds the ability to generate a sensor from a list observable_source_asset declarations.

The goal here is to demonstrate and have a story for executing observations within sensors when we consolidate @observable_source_asset and into the mainline AssetsDefinition ontology by having the compute function able to generate an observation, not just a materialization.

So now without having to opt-in to AMPs, you can easily generate a sensor to refresh source assets at a regular cadence without having the write a sensor. observable_source_asset will become a deprecated API in this world, almost certainly.

In a test case we have the following code:

defs = Definitions(
    assets=[external_asset_from_source_asset(an_asset)],
    sensors=[sensor_def_from_observable_source_assets([an_asset])],
)

If we so chose, we could push down calling those functions against the source asset to within the Definitions __init__ method machinery to make this completely automatic. Whether or not that is a good idea is a different discussion.

How I Tested These Changes

BK

[schrockn]

• Make observable source adapter layer compatibility with AMP #16718
• @observable_source_asset adapter to generate and SensorDefinition #16716 👈
• Add a new AssetExecutionType (Observable) and use it to omit materializations for observable source asset wrapping #16621 : 2 other dependent PRs (#16623 , #16712 )
• Wrap observable source assets in AssetsDefinition #16620 : 1 other dependent PR (#17063 )
• Extract _get_op_def_compute_fn into wrap_source_asset_observe_fn_in_op_compute_fn #16618 : 1 other dependent PR (#16619 )
• Source Asset -> Unexecutable Asset Adapter #16617
• master

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @schrockn and the rest of your teammates on Graphite

[schrockn]

@sryza relevant test case here

[schrockn]

@OwenKephart I was trying to find the default value for auto_observe_interval_minutes. I couldn't find it in the codebase. I'll update the docblock for SourceAsset once I know what it is.

[OwenKephart]

There's no default value for this (other than `None) -- if someone wants an observable source asset to be automatically observed they need to pass in an explicit number.

5 minutes seems like a fine default here, although I think I'm missing some context for the overall PR. Tying this sensor's frequency to the value set on auto_observe_interval_minutes would require that we also turn off any auto-observing functionality in the asset daemon to avoid two daemons trying to do the same job.

The machinery for auto-observing source assets is very different from the machinery for auto-materializing regular assets (for one, there's no system of rules for when source assets should be observed, it's just based on elapsed time), so in general I think it's reasonable to model this as just traditional sensor if we don't think there will be any need for more complex logic for when to kick these off (I'd buy that).

Some bits of trepidation:

1. we'd probably want to rename this property to something that's less similar to auto_materialize. as proposed, this is less declarative and "automatic" than the existing solution (as you need to add code in two places in order for your assets to actually be observed automatically). as you mention, this could be fixed by adding things to the Definitions machinery.
2. for users with tons of observable source assets, it might be offputting to end up with an equal number of sensors. this might be something that clever UI could handle (potentially grouping together all these auto-generated sensors).

[schrockn]

for users with tons of observable source assets, it might be offputting to end up with an equal number of sensors. this might be something that clever UI could handle (potentially grouping together all these auto-generated sensors).

Absolutely. We'll want a version of this which can take multiple observable source assets. In fact @OwenKephart I think you should request changes here and then I should do exactly that.

we'd probably want to rename this property to something that's less similar to auto_materialize. as proposed, this is less declarative and "automatic" than the existing solution (as you need to add code in two places in order for your assets to actually be observed automatically). as you mention, this could be fixed by adding things to the Definitions machinery.

We talked about this live, but the goal here is to provide a backwards compatibility layer. Agree that any new APIs should avoid the language.

[alangenfeld]

you should request changes here

phabricator plan changes , i miss you so

[schrockn]

phabricator plan changes , i miss you so

Miss phab every day.